Skip to content

fix: restore background Chromium cookie imports without Keychain prompts - #2611

Closed
tangzhu599 wants to merge 1 commit into
steipete:mainfrom
tangzhu599:fix/background-chromium-cookie-imports
Closed

fix: restore background Chromium cookie imports without Keychain prompts#2611
tangzhu599 wants to merge 1 commit into
steipete:mainfrom
tangzhu599:fix/background-chromium-cookie-imports

Conversation

@tangzhu599

Copy link
Copy Markdown

Summary

Restore automatic Chromium cookie imports for background refreshes without reintroducing Keychain prompts. Cookie-only providers such as Kimi currently report "No available fetch strategy for kimi." on every automatic refresh and only work after a manual refresh.

Root cause

#2225 (17c8729) blocked all background Chromium cookie access by returning false from BrowserCookieAccessGate.shouldAttempt whenever the interaction context is not .userInitiated. Periodic and menu-open refreshes run in .background, so a provider whose only credential path is browser cookies (no API key, no fresh CLI credential) reports noAvailableStrategy until the user performs a manual (.userInitiated) refresh. The #2225 review even flagged this: "users who rely on Chromium cookies for automatic provider refresh may receive stale usage or authentication failures until they perform a manual refresh."

#2225's own follow-up prescribed the fix: SweetCookieKit was hardened so hosts can suppress interactive Safe Storage promotion, and "once released, CodexBar should bump that dependency."

Change

  • SweetCookieKit 0.4.1 → 0.5.1 — adds BrowserCookieKeychainAccessGate.withUserInteractionDisallowed, the hardening Block background Chromium Keychain access #2225 planned for.
  • BrowserCookieAccessGate.shouldAttempt — background path now: honors recorded denial cooldowns, runs the prompt-free attribute preflight (skips when interaction is required), and allows the import otherwise, instead of blanket-skipping every Chromium browser.
  • codexBarRecords — every cookie read goes through withCookieReadInteractionPolicy: background reads run with withUserInteractionDisallowed (SweetCookieKit can never promote to an interactive prompt; a key that needs UI simply yields no cookies), user-initiated reads keep the interactive recovery path.
  • recordDenied — only user-initiated actions write denial cooldowns, so a background probe can never suppress the user's next manual refresh for 6h.
  • Tests updated for the new background contract; regression tests added for the no-cooldown-poisoning and non-interactive-read guarantees.

Safety

  • No background path can surface a Safe Storage prompt: preflight is non-interactive, and the read itself is wrapped with withUserInteractionDisallowed.
  • Background probes never write denial cooldowns (a background probe must not suppress the user's next manual refresh).
  • Safari / Firefox / Zen behavior unchanged; user-initiated Chromium behavior unchanged.

Validation

  • swift build --target CodexBarCore and CodexBarCLI pass.
  • Gate behavior verified against the real module with a scratch harness: 10/10 checks pass (background admits Chromium when the preflight allows, background reads disallow keychain interaction, background probes don't poison manual refresh, user-initiated denial/cooldown semantics preserved).
  • SwiftFormat clean. Full Swift Testing suite runs in CI.

Background refreshes skipped every Chromium browser (17c8729) to avoid Safe Storage
prompts, which broke cookie-only providers like Kimi ("No available fetch strategy
for kimi") until a manual refresh. Bump SweetCookieKit to 0.5.1 and run background
cookie reads with withUserInteractionDisallowed: browsers whose Safe Storage key
requires interaction yield no cookies instead of prompting, the prompt-free
attribute preflight still filters locked/denied keychain states, and background
probes never write denial cooldowns that would suppress the next manual refresh.
@clawsweeper

clawsweeper Bot commented Aug 3, 2026

Copy link
Copy Markdown

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix: restore background Chromium cookie imports without Keychain prompts This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f86068d904

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// Only user-initiated actions may write denial cooldowns. Background probes must
// not suppress a browser for hours (including the user's next manual refresh)
// based on a non-interactive probe outcome.
guard ProviderInteractionContext.current == .userInitiated else { return }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update direct cooldown seeds for the interaction guard

Because ProviderInteractionContext.current defaults to .background, this guard makes every existing unwrapped recordDenied test seed a no-op. Multiple macOS tests still rely on those calls, including OllamaUsageFetcherTests.swift:170, which immediately expects hasActiveDenial to be true, and CLICookieRefreshTests.swift:305, which expects a stored-denial recovery hint; the full test suite will therefore fail. Wrap all intended user-denial seeds in a .userInitiated context or provide a dedicated test-state helper.

AGENTS.md reference: AGENTS.md:L23-L24

Useful? React with 👍 / 👎.

@tangzhu599

Copy link
Copy Markdown
Author

Closing as duplicate: upstream #2528 (a388338) already implements the same fix — background refreshes reuse an already-authorized Safe Storage ACL. This branch also carried two hardening deltas (read-site withUserInteractionDisallowed wrapper, userInitiated-only denial recording) that can be re-proposed as a focused follow-up if wanted.

@tangzhu599 tangzhu599 closed this Aug 3, 2026
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