-
Notifications
You must be signed in to change notification settings - Fork 1
Fix CDP reconnect when Chrome has zero open tabs #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
12fc078
8fd0fc9
5e110ca
5dfd437
df28518
38a0b03
499f312
c8ab0ad
5f1f991
27d4430
75f1da6
2241937
7c12eb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 1.0.0.6 | ||
| 1.0.0.7 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -160,23 +160,13 @@ fi | |
| BROWSER_SVC="${REPO_ROOT}/bin/browser-service" | ||
| if [[ -f "$BROWSER_SVC" ]]; then | ||
| chmod +x "$BROWSER_SVC" 2>/dev/null || true | ||
| chrome_bin="" | ||
| mac_chrome="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" | ||
| if [[ -x "$mac_chrome" ]]; then | ||
| chrome_bin="$mac_chrome" | ||
| elif command -v google-chrome >/dev/null 2>&1; then | ||
| chrome_bin="$(command -v google-chrome)" | ||
| elif command -v chromium >/dev/null 2>&1; then | ||
| chrome_bin="$(command -v chromium)" | ||
| elif command -v chromium-browser >/dev/null 2>&1; then | ||
| chrome_bin="$(command -v chromium-browser)" | ||
| fi | ||
| if [[ -n "$chrome_bin" ]]; then | ||
| if OUTREACH_REPO_ROOT="$REPO_ROOT" CHROME_BIN="$chrome_bin" "$BROWSER_SVC" install; then | ||
| echo "[outreach-upgrade] Refreshed browser auto-start (bin/browser-service install)" | ||
| else | ||
| echo "[outreach-upgrade] bin/browser-service install skipped (no launchd/systemd)" >&2 | ||
| fi | ||
| # Let browser-service resolve_chrome pick the binary (Playwright's | ||
| # Chromium first, then system Chrome) instead of forcing system Chrome | ||
| # here — forcing it would undo that preference on every upgrade. | ||
| if OUTREACH_REPO_ROOT="$REPO_ROOT" "$BROWSER_SVC" install; then | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed guard for "no Chrome binary found at all" changes the failure message, not just the Chrome-choice logic. The old code only called |
||
| echo "[outreach-upgrade] Refreshed browser auto-start (bin/browser-service install)" | ||
| else | ||
| echo "[outreach-upgrade] bin/browser-service install failed (see warning above)" >&2 | ||
| fi | ||
| fi | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -537,7 +537,10 @@ launch_chrome_cdp() { | |
|
|
||
| export OUTREACH_REPO_ROOT="${REPO_ROOT}" | ||
| export CDP_PORT CHROME_PROFILE | ||
| export CHROME_BIN="${chrome}" | ||
| # Don't export CHROME_BIN here — resolve_chrome (bin/browser-service) picks | ||
| # Playwright's bundled Chromium first, falling back to this same system | ||
| # Chrome if that's unavailable. Forcing it here would skip that preference | ||
| # on every fresh install, not just reintroduce it on upgrades. | ||
|
|
||
| info "Running bin/browser-service install…" | ||
| if ! "${svc}" install; then | ||
|
|
@@ -628,6 +631,20 @@ prompt_linkedin_login() { | |
| fi | ||
| } | ||
|
|
||
| persona_already_configured() { | ||
| local persona="${REPO_ROOT}/outreach/config/persona.json" | ||
| local example="${REPO_ROOT}/outreach/config/persona.json.example" | ||
| [[ -f "${persona}" ]] || return 1 | ||
| [[ -f "${example}" ]] && cmp -s "${persona}" "${example}" && return 1 | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No corruption/validity check before keeping an existing persona.json.
|
||
| # ponytail: treat unparseable JSON (e.g. truncated by an interrupted prior | ||
| # install) as unconfigured so it gets re-synced instead of kept forever. | ||
| # Skips the check if python3 isn't on PATH yet. | ||
| if command -v python3 >/dev/null 2>&1; then | ||
| python3 -c 'import json,sys; json.load(open(sys.argv[1]))' "${persona}" >/dev/null 2>&1 || return 1 | ||
| fi | ||
| return 0 | ||
| } | ||
|
|
||
| run_sync_planner_persona_skill() { | ||
| local prompt="Run the sync-planner-persona-from-linkedin skill for profile ${PERSONA_PROFILE_URL}" | ||
| local rerun_cmd="cd \"${REPO_ROOT}\" && claude -p '${prompt}'" | ||
|
|
@@ -637,6 +654,27 @@ run_sync_planner_persona_skill() { | |
| note "skip: planner persona sync (--skip-persona-sync)" | ||
| return 0 | ||
| fi | ||
|
|
||
| if persona_already_configured; then | ||
| if [[ -t 0 ]]; then | ||
| local reply="" | ||
| info "outreach/config/persona.json is already configured (reinstall detected)." | ||
| read -r -p "[install] Re-sync persona from LinkedIn and overwrite it? [y/N] " reply || reply="" | ||
| case "${reply}" in | ||
| y|Y|yes|YES) ;; | ||
| *) | ||
| info "Keeping existing outreach/config/persona.json." | ||
| note "ok: existing persona.json kept (reinstall)" | ||
| return 0 | ||
| ;; | ||
| esac | ||
| else | ||
| info "Non-interactive reinstall — keeping existing outreach/config/persona.json." | ||
| info "To re-sync: ${rerun_cmd}" | ||
| note "ok: existing persona.json kept (non-interactive reinstall)" | ||
| return 0 | ||
| fi | ||
| fi | ||
| if ! command -v claude >/dev/null 2>&1; then | ||
| info "claude CLI not on PATH — skipping planner persona sync." | ||
| info "After installing Claude Code, run: ${rerun_cmd}" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,6 +69,8 @@ | |
| import random | ||
| import re | ||
| import uuid | ||
| import urllib.error | ||
| import urllib.request | ||
| from urllib.parse import urlparse | ||
| from datetime import datetime, timezone | ||
| from pathlib import Path | ||
|
|
@@ -79,6 +81,7 @@ | |
| from playwright.async_api import ( | ||
| Browser, | ||
| BrowserContext, | ||
| Error, | ||
| Locator, | ||
| Page, | ||
| Playwright, | ||
|
|
@@ -528,6 +531,31 @@ def _pp_structure_activity_update(index: int, post: dict[str, Any]) -> dict[str, | |
|
|
||
| # ── Browser wrapper ─────────────────────────────────────────────────────────── | ||
|
|
||
| # ponytail: matched by substring against the real Chrome zero-open-tabs CDP | ||
| # error; if Chrome's wording changes, this stops matching and _attach falls | ||
| # back to re-raising instead of retrying — update the substring then. | ||
| _ZERO_TAB_CDP_ERROR = "setDownloadBehavior" | ||
|
|
||
|
|
||
| def _open_blank_tab(cdp_url: str) -> None: | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reimplements a workaround that already exists in install.sh.
|
||
| """Ask Chrome's CDP HTTP endpoint to open a tab (bypasses Playwright context creation). | ||
|
|
||
| install.sh's open_linkedin_tab_in_cdp() does the same PUT-then-GET | ||
| workaround in bash for the pre-attach case (no Python env to call into | ||
| yet) — keep both in sync if Chrome's CDP behavior here changes. | ||
| """ | ||
| req = urllib.request.Request(f"{cdp_url}/json/new", method="PUT") | ||
| try: | ||
| urllib.request.urlopen(req, timeout=5).close() | ||
| except urllib.error.HTTPError: | ||
| # Older Chrome builds reject PUT on this endpoint (e.g. 404/405) but | ||
| # accept GET. A plain URLError (timeout, connection refused) means | ||
| # the request may already have gone through server-side, or Chrome | ||
| # isn't reachable at all — retrying here would risk opening a | ||
| # second tab, so only HTTPError (a real "this doesn't work" reply) | ||
| # triggers the GET fallback. | ||
| urllib.request.urlopen(f"{cdp_url}/json/new", timeout=5).close() | ||
|
|
||
|
|
||
| class LinkedInBrowser: | ||
| """ | ||
|
|
@@ -624,6 +652,34 @@ async def _launch(self) -> None: | |
| # Inject stealth overrides on every new page / frame. | ||
| await self._ctx.add_init_script(_STEALTH_SCRIPT) | ||
|
|
||
| async def _connect_with_zero_tab_retry(self) -> Browser: | ||
| """ | ||
| Connect over CDP, working around Chrome's zero-open-tabs quirks. | ||
|
|
||
| With zero open tabs, real Chrome's connect_over_cdp either throws | ||
| _ZERO_TAB_CDP_ERROR outright (it has no default context to attach | ||
| to), or — on some builds — succeeds but reports an empty contexts | ||
| list. Either way, asking Chrome's own CDP HTTP endpoint to open a | ||
| blank tab and reconnecting fixes it, so both cases retry the same | ||
| way. | ||
| """ | ||
| for attempt in (1, 2): | ||
| try: | ||
| browser = await self._pw.chromium.connect_over_cdp(self.cdp_url) | ||
| except Error as e: | ||
| if attempt == 2 or _ZERO_TAB_CDP_ERROR not in str(e): | ||
| raise | ||
| browser = None | ||
| if browser is not None and browser.contexts: | ||
| return browser | ||
| if attempt == 2: | ||
| raise RuntimeError( | ||
| f"Chrome at {self.cdp_url} reported no browser context " | ||
| "even after opening a tab via its CDP HTTP endpoint." | ||
| ) | ||
| await asyncio.to_thread(_open_blank_tab, self.cdp_url) | ||
| raise AssertionError("unreachable") # loop always returns or raises | ||
|
|
||
| async def _attach(self) -> None: | ||
| """ | ||
| Attach to an already-running Chrome via CDP and pick the best tab to use. | ||
|
|
@@ -638,15 +694,11 @@ async def _attach(self) -> None: | |
|
|
||
| We never close a tab we didn't open, so the user's browsing is undisturbed. | ||
| """ | ||
| self._browser = await self._pw.chromium.connect_over_cdp(self.cdp_url) | ||
| self._browser = await self._connect_with_zero_tab_retry() | ||
| self._is_attached = True | ||
|
|
||
| # Inherit the real user session (cookies, localStorage, etc.). | ||
| self._ctx = ( | ||
| self._browser.contexts[0] | ||
| if self._browser.contexts | ||
| else await self._browser.new_context() | ||
| ) | ||
| self._ctx = self._browser.contexts[0] | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possible IndexError if contexts is still empty after the retry.
|
||
|
|
||
| page = self._pick_tab(self._ctx.pages) | ||
| if page is not None: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolve_playwright_chromiumspawns a fulluv run+ Python + Playwright-import subprocess with no caching.This runs on every
resolve_chromecall, i.e. everyinstall/daemon/startinvocation — including launchd/systemdKeepAliverespawns. A Chrome crash-loop now also crash-loops this subprocess each cycle. Since the resolved path only changes when Playwright's browser is (re)installed, worth caching it (e.g. a file next toREPO_ROOTinvalidated by playwright version, or just resolving once atinstalltime and relying on theCHROME_BINalready baked into the plist/unit).