Skip to content

Try each browser session for Factory - #916

Merged
Finesssee merged 5 commits into
mainfrom
fix/factory-browser-sessions
Oct 19, 2026
Merged

Finesssee merged 5 commits into
mainfrom
fix/factory-browser-sessions

Conversation

@Finesssee

@Finesssee Finesssee commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Factory web usage now tries each browser's session in the shared browser order and moves on to the next browser when Factory rejects a session (401/403). Before, the first browser holding any Factory cookie was the only one tried, so a stale session in that browser hid a valid login in a later browser.

Behavior changes:

  • Browser cookie import (cookie source Auto, Browser or Web) reads browsers one at a time, in the shared browser order, and stops at the first session Factory accepts. Browsers after that session are not read.
  • A rejected session (AuthRequired) moves on to the next browser. When every session is rejected, the provider reports AuthRequired (sign-in needed), as before for a single browser.
  • Any other error (network, 5xx, parse) from a session stops the search and is reported, not replaced by a later browser's result.
  • A pasted or stored manual Cookie header is still used alone and never reads a browser.
  • Cookie source Off, or an empty Manual source, reads no browser and makes no web request. No browser holding a Factory cookie reports NoCookies.

Routing fix: the shell used to pre-resolve the first browser's Factory cookie into FetchContext::manual_cookie_header for Auto, Browser and Web. fetch_via_web then used that one header and the per-browser loop never ran. FactoryProvider now overrides owns_browser_cookie_resolution() to return true, the same pattern as Zed, so the shell defers browser discovery to the provider. Stored and pasted cookies still pass through unchanged.

Implementation: rust/src/providers/factory/mod.rs uses browser_cookie_sessions (the #905 BrowserCookieSessions helper) and a fetch_web_sessions loop that follows the cursor, zed and notion loops. The HTTP base is a parameter so tests can use a mock server.

Upstream reference: upstream v0.76.0 steipete#4461 (9ebd90017f2c216528ae3ac201599cda0516d855, "fix(factory): import sessions from every Firefox channel") covers the Factory browser order, Firefox Beta / Developer Edition / Nightly imports, and not aborting when a browser has no readable cookie store. It does not cover moving on after a rejected session, which is this port's change. This port has one Firefox browser type. Its profile scan treats any profile name containing default as the default profile (rust/src/browser/detection.rs), so Developer Edition profiles are found. Separate Beta, Developer Edition and Nightly browser types are not added here.

Divergence from upstream, deliberate: upstream continues to the next session on any per-session fetch error. This port continues only on a rejected session, to match the shared #905 pattern used by the other self-resolving cookie providers on Win-CodexBar. Network or server errors therefore stop the search here.

Tests

Provider tests in rust/src/providers/factory/tests.rs (in-memory browser store, mockito Factory host, no real browser read):

  • rejected_browser_session_moves_on_to_the_next_browser: Chrome 401, Edge 200 gives success; Firefox is never read.
  • forbidden_browser_session_also_moves_on: Chrome 403, then Edge is accepted.
  • every_rejected_browser_session_reports_auth_required: Chrome 401 and Edge 401 give AuthRequired.
  • server_error_from_a_browser_session_stops_the_search: Chrome 500 gives Other; Edge is never requested.
  • no_browser_cookies_reports_no_cookies_without_a_request: empty store gives NoCookies.
  • factory_owns_browser_cookie_resolution: the provider reports the override.
  • missing_manual_cookie_gate_reports_no_cookies_without_any_request: with manual_cookie_missing set and two browsers holding cookies, the result is NoCookies, with zero auth and usage requests (mockito expect(0)) and no browser store read.

Shell tests in apps/desktop-tauri/src-tauri/src/commands/fetch_context_tests.rs:

  • factory_browser_cookie_sources_leave_session_choice_to_the_provider: cookie source Auto, Browser or Web gets no pre-resolved header and is not marked missing.
  • factory_stored_cookie_passes_through_unchanged: a stored cookie is forwarded unchanged for Auto and Manual.

Commands run

On the merged head (after merging origin/main):

  • cargo fmt --all -- --check: pass
  • cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings: clean
  • cargo clippy --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml --all-targets -- -D warnings: clean
  • cargo test --manifest-path rust/Cargo.toml factory: 22 passed, 0 failed
  • cargo test --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml fetch_context: 11 passed, 0 failed

Not run in this pass: the full cargo test for either manifest.

Not UI-affecting: the change is provider fetch logic plus one CHANGELOG bullet, so no CUA proof is required.

Summary by CodeRabbit

  • Bug Fixes
    • Factory sign-in can now continue to another browser when a browser session is rejected as signed out or expired.
    • Server errors stop the browser search, and browsers after an accepted session are not checked.
    • A manually pasted Cookie header remains the only credential source when provided; no available browser cookies results in a clear sign-in error.

Factory web usage now tries each browser session in the shared browser order and moves on when Factory rejects one (401/403), instead of stopping at the first browser that held any Factory cookie. A pasted Cookie header is still used alone, and other errors still stop the search. Ports the upstream v0.76.0 steipete#4461 behavior through the shared BrowserCookieSessions helper from #905.
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: nesszer/Win-CodexBar/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 10d597dd-1dd5-4c39-a905-084ab557a51a
📥 Commits

Reviewing files that changed from the base of the PR and between 1bfe3e9 and 2120fe9.

📒 Files selected for processing: 4
  • CHANGELOG.md
  • apps/desktop-tauri/src-tauri/src/commands/fetch_context_tests.rs
  • rust/src/providers/factory/mod.rs
  • rust/src/providers/factory/tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Factory web requests now try browser sessions in order when no nonblank manual cookie is supplied. Authentication rejections advance to the next session; other results stop the search. Added tests cover session fallback, exhaustion, errors, and cookie-source handling.

Changes: Factory browser-cookie fallback

Layer / File(s) Summary
Resolve and try Factory browser sessions
rust/src/providers/factory/mod.rs, rust/src/providers/factory/tests.rs, CHANGELOG.md
FactoryProvider uses browser sessions when no nonblank manual cookie is available. It advances after AuthRequired and returns other results without trying later sessions. Tests cover rejected sessions, exhaustion, server errors, missing cookies, and avoiding reads after success or when the manual cookie is missing. The changelog records this behavior.
Verify Factory cookie-source handling
apps/desktop-tauri/src-tauri/src/commands/fetch_context_tests.rs
Tests verify that Factory leaves browser-session resolution to the provider for auto, browser, and web sources, and forwards stored cookies unchanged for auto and manual sources.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant FactoryProvider
  participant BrowserCookieSessions
  participant BrowserCookieStore
  participant FactoryAPI
  FactoryProvider->>BrowserCookieSessions: discover sessions for Factory domains
  BrowserCookieSessions->>BrowserCookieStore: read browser cookies
  BrowserCookieStore-->>BrowserCookieSessions: ordered browser sessions
  BrowserCookieSessions-->>FactoryProvider: sessions
  loop Each session until a non-authentication result
    FactoryProvider->>FactoryAPI: request with session cookie
    FactoryAPI-->>FactoryProvider: AuthRequired or other result
  end
Loading
Merge Risk: ⚪ Minimal · up to 2120f

No merge-blocking issue was identified; the change is ready for normal checks.

Pre-merge checks: Passed 8
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title is a short imperative summary that accurately describes the main change: trying each browser session for Factory.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Provider Data Stays Siloed Passed The reviewed changes stay within Factory provider logic, Factory tests, fetch-context tests, and the changelog. FactoryProvider uses generic BrowserCookieSessions and only handles Factory-specific…
Secrets Handled Safely Passed No unsafe secret handling was introduced. The changed Factory code sends cookies only as HTTP Cookie headers through the existing browser-session helper and does not log or include them in errors. N…
No Unapproved Dependencies Passed The pull request changes only CHANGELOG.md and Rust source/test files. The authoritative diff contains no Cargo.toml, package.json, npm/yarn lockfile, or pnpm-lock.yaml changes. Therefore, it adds no …
Ui Changes Include Windows Proof Passed The pull request changes only CHANGELOG.md, backend Factory provider code, and tests. It does not change files under apps/desktop-tauri/src, rust/src/tray, the float bar, settings, or window chrome. T…
Full details: Provider Data Stays Siloed

Explanation

The reviewed changes stay within Factory provider logic, Factory tests, fetch-context tests, and the changelog. FactoryProvider uses generic BrowserCookieSessions and only handles Factory-specific cookies and Factory responses. It does not transfer identity, plan, account label, or email to another provider or account. No changed shared production code adds branching on a specific ProviderId; the ProviderId::Factory references are test inputs. The provider's email and organization fields remain Factory response data used in the Factory usage snapshot.

Full details: Secrets Handled Safely

Explanation

No unsafe secret handling was introduced. The changed Factory code sends cookies only as HTTP Cookie headers through the existing browser-session helper and does not log or include them in errors. New error text contains only HTTP status or generic authentication messages. No credential files or keyring entries are deleted or overwritten. Test cookie values are deterministic fixtures, not real credentials, and no snapshot is added.

Full details: No Unapproved Dependencies

Explanation

The pull request changes only CHANGELOG.md and Rust source/test files. The authoritative diff contains no Cargo.toml, package.json, npm/yarn lockfile, or pnpm-lock.yaml changes. Therefore, it adds no dependency and does not change the pinned pnpm packageManager version.

Full details: Ui Changes Include Windows Proof

Explanation

The pull request changes only CHANGELOG.md, backend Factory provider code, and tests. It does not change files under apps/desktop-tauri/src, rust/src/tray, the float bar, settings, or window chrome. The UI-proof requirement is therefore not applicable.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Factory Agent added 4 commits October 19, 2026 02:04
Factory now overrides owns_browser_cookie_resolution, so the shell no longer pins the first browser's cookie header into the fetch context for auto, browser, or web. Before this, a pre-resolved header made fetch_via_web use only that browser and skip the per-browser session loop. Adds provider and shell tests for the override, the missing-cookie gate with zero requests, and stored-cookie passthrough.
Drop the Firefox Beta, Developer Edition, and Nightly claim, which has no matching browser variants in this port, and the upstream steipete#4461 attribution, which covers Firefox channel imports rather than moving on after a rejected session.
@Finesssee
Finesssee merged commit 7a89115 into main Oct 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant