Skip to content

fix(#222): capture /fx-scoped session cookie + seed headed context from pre-launch snapshot (macOS) - #230

Merged
ffroliva merged 1 commit into
ffroliva:developfrom
gunalak:fix/222-macos-seed-on-develop
Jul 1, 2026
Merged

ffroliva merged 1 commit into
ffroliva:developfrom
gunalak:fix/222-macos-seed-on-develop

Conversation

@gunalak

@gunalak gunalak commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #222 — macOS generation 401

Re: dropping the seed as "disproven" in 98bd455 — I think the seed actually works; the dropped version just sourced cookies from browser_cookie3 (which fails on macOS) and read them at path /. With two corrections, generation succeeds on macOS (Apple Silicon, verified end-to-end).

1. Read flow cookies by domain, not root-path URL (unconditional bug fix)

_get_chrome_cookies_playwright used ctx.cookies(["https://labs.google"]), which only returns cookies whose path matches / — silently dropping __Secure-next-auth.session-token (scoped to /fx, where the Flow app + createProject live). Switched to the full jar filtered by domain. This alone fixes verification / pre-read capturing the session (preread_count went 2 → 5–8, now including the session token).

2. Pre-read + seed the session into the headed context (safety net)

On macOS the headed generation context's native cookie decrypt is intermittent:

  • sometimes it loads 0 cookies (context_cookie_count=0, flow_session_cookie_present=False) → createProject 401s;
  • sometimes it loads them fine (context_cookie_count=7, present=True).

When it loads nothing, we seed the session from a snapshot read pre-launch via the headless --password-store=basic reader (the one path that decrypts on macOS; done pre-launch to avoid the profile singleton-lock deadlock). Restores _ensure_context_session_cookie (log + seed). No-op where the context loads the cookie itself (Windows, or the successful-decrypt runs).

Verified on macOS (Apple Silicon) — both paths

# seed fires (headed decrypt loaded nothing):
context_cookie_count=0  flow_session_cookie_present=False
preread_count=8  preread_session=True
context_cookies_seeded  seeded_session=True   → createProject + batchGenerateImages 200 → image written

# native decrypt (seed is a no-op):
context_cookie_count=7  flow_session_cookie_present=True   → 200 → image written

Full suite: 1906 passed, 6 skipped, 1 xfailed. Added 3 tests for the seed path.

Separate finding (worth its own follow-up)

The 401 was also masked by a login-capture bug: on macOS, if Chrome is already running, gflow auth login --browser chrome is hijacked by the running instance and the sign-in lands in the user's default profile, so the session is never persisted to the gflow profile (the profile had __Host-next-auth.csrf-token + callback-url but no session-token/SAPISID). Quitting Chrome before auth login fixed capture. gflow could detect a running Chrome at login and warn/relaunch — happy to file separately.

…om pre-launch snapshot (macOS)

Two macOS-specific fixes so generation authenticates (createProject no longer 401s):

1. cookies.py: _get_chrome_cookies_playwright derives flow cookies from the FULL
   jar filtered by domain, not ctx.cookies([labs.google]) which only returns
   path-"/" cookies and silently dropped __Secure-next-auth.session-token
   (scoped to /fx). This is the unconditional bug — verification/pre-read now
   capture the session (preread_count 2 -> 5-8).

2. client.py: restore _ensure_context_session_cookie (log + seed) and pre-read
   the flow cookies via get_chrome_cookie_snapshot BEFORE the headed context
   launches (the headless --password-store=basic fallback is the one reader that
   decrypts on macOS; pre-launch avoids the profile singleton-lock deadlock).
   Seed them into the headed context when its native decrypt loaded nothing.

On macOS the headed context's native decrypt is intermittent: observed both
context_cookie_count=0 (seed rescues -> seeded_session=True) and =7 (native,
seed no-op). Both reach batchGenerateImages 200. No-op on Windows.

Adds 3 tests for the seed path. Full suite: 1906 passed, 6 skipped, 1 xfailed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gunalak
gunalak requested a review from ffroliva as a code owner July 1, 2026 04:29
@github-actions github-actions Bot added external-contribution Pull request from a non-maintainer or fork needs-copilot-review Ask GitHub Copilot for advisory code review needs-maintainer-review Requires maintainer review before merge labels Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Thanks for the contribution. This PR has been marked as an external contribution and routed for maintainer review.

Before merge, please make sure:

  • The PR targets develop.
  • Commits use a real Git identity or GitHub noreply email.
  • External commits include a DCO sign-off: git commit -s.
  • The PR does not include secrets, cookies, account tokens, signed URLs, or private captured data.
  • Focused tests/docs are included for behavior changes.

GitHub Copilot code review may be requested as an advisory first pass, but maintainer approval is still required.

@ffroliva
ffroliva merged commit c647a35 into ffroliva:develop Jul 1, 2026
1 check passed
@ffroliva ffroliva mentioned this pull request Jul 1, 2026
ffroliva added a commit that referenced this pull request Jul 1, 2026
The #228 boundary guards pushed gflow_generate_image/video over the cognitive-
complexity threshold and duplicated the bad-parameter error literal. Extract:

- `_BAD_PARAM_TYPE` constant + `_bad_param()` builder (fixes S1192 duplicated literal)
- `_resolve_image_references()` and `_build_video_media_inputs()` helpers, moving
  the mode/mutual-exclusion/path validation out of the tool functions (fixes S3776
  cognitive complexity on both)

Adds unit tests for the extracted helpers plus the #230 `_preread_flow_session_cookies`
capture path (D4 council gap) and the video `tool_specs` expansion branch — lifting
new-code coverage back over 80%. Behavior-preserving; 38 affected tests green,
pyright src 0 errors.
ffroliva added a commit that referenced this pull request Jul 1, 2026
test(222): falsifiable /fx cookie-path regression for #230
@gunalak
gunalak deleted the fix/222-macos-seed-on-develop branch July 1, 2026 11:48
kittinan pushed a commit to kittinan/gflow-cli that referenced this pull request Jul 1, 2026
The ffroliva#230 fix reads the full cookie jar filtered by domain instead of the
path-/ URL filter that silently dropped the /fx-scoped
__Secure-next-auth.session-token. This regression asserts the session token
survives _get_chrome_cookies_playwright: the mocked context returns it only
from the full jar (ctx.cookies()), not the URL-filtered read
(ctx.cookies([url])), so the test FAILS on the pre-ffroliva#230 form and PASSES on the
fix (verified by temporary revert). Closes the D4 council coverage gap.
15990166686ll-crypto pushed a commit to 15990166686ll-crypto/gflow-cli that referenced this pull request Aug 24, 2026
…velop

fix(ffroliva#222): capture /fx-scoped session cookie + seed headed context from pre-launch snapshot (macOS)
15990166686ll-crypto pushed a commit to 15990166686ll-crypto/gflow-cli that referenced this pull request Aug 24, 2026
The ffroliva#228 boundary guards pushed gflow_generate_image/video over the cognitive-
complexity threshold and duplicated the bad-parameter error literal. Extract:

- `_BAD_PARAM_TYPE` constant + `_bad_param()` builder (fixes S1192 duplicated literal)
- `_resolve_image_references()` and `_build_video_media_inputs()` helpers, moving
  the mode/mutual-exclusion/path validation out of the tool functions (fixes S3776
  cognitive complexity on both)

Adds unit tests for the extracted helpers plus the ffroliva#230 `_preread_flow_session_cookies`
capture path (D4 council gap) and the video `tool_specs` expansion branch — lifting
new-code coverage back over 80%. Behavior-preserving; 38 affected tests green,
pyright src 0 errors.
15990166686ll-crypto pushed a commit to 15990166686ll-crypto/gflow-cli that referenced this pull request Aug 24, 2026
The ffroliva#230 fix reads the full cookie jar filtered by domain instead of the
path-/ URL filter that silently dropped the /fx-scoped
__Secure-next-auth.session-token. This regression asserts the session token
survives _get_chrome_cookies_playwright: the mocked context returns it only
from the full jar (ctx.cookies()), not the URL-filtered read
(ctx.cookies([url])), so the test FAILS on the pre-ffroliva#230 form and PASSES on the
fix (verified by temporary revert). Closes the D4 council coverage gap.
15990166686ll-crypto pushed a commit to 15990166686ll-crypto/gflow-cli that referenced this pull request Aug 24, 2026
…-regression

test(222): falsifiable /fx cookie-path regression for ffroliva#230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contribution Pull request from a non-maintainer or fork needs-copilot-review Ask GitHub Copilot for advisory code review needs-maintainer-review Requires maintainer review before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macOS: generation runs on bundled Playwright Chromium despite .gflow_browser_strategy=chrome → 401 AuthExpiredError on createProject

2 participants