Skip to content

Port upstream 0.67.0: credential expiry alerts - #660

Closed
Finesssee wants to merge 2 commits into
port/upstream-0.67.0from
port/micro-0.67.0-credential-expiry-alerts
Closed

Finesssee wants to merge 2 commits into
port/upstream-0.67.0from
port/micro-0.67.0-credential-expiry-alerts

Conversation

@Finesssee

Copy link
Copy Markdown
Collaborator

Summary

Adds opt-in credential-expiry alerts. New setting credential_expiry_notifications_enabled (default off, toggle under Settings > Notifications, "Credential Expiry Alerts").

When a provider refresh fails with a typed sign-in state (ProviderStateKind::NeedsAuthentication or ExpiredSession, taken from Provider::error_state_kind, so providers whose NotInstalled means a missing local runtime do not alert), the first failure for a (provider, account scope) posts one toast: " needs sign-in" with the generic body "Open CodexBar to review the account error and sign in again." The toast never contains the raw error, email or account id.

  • One alert per failure episode. Repeated failures, intervening network/timeout/parse/quota/permission/rate-limit failures, and cached or last-good fallback snapshots neither end nor restart it. The state kind is captured from the fresh fetch before last-good preservation, so a replayed snapshot cannot count as recovery.
  • Only a fresh successful fetch (no error) for the same account scope ends an episode. Recovery is recorded even while alerts are off.
  • Episodes are in memory (reset on restart). Turning the toggle off suppresses delivery and does not forget open episodes; a failure that occurs while off does not start an episode.
  • A toast that cannot be dispatched (PowerShell spawn failure) releases its reservation so the next refresh retries.
  • Account scope reuses quota_notification_account_identity (token-account id, then email, then org). A failed fetch has no identity of its own, so the identity of the last good snapshot is used. An empty scope (no evidence) is a wildcard for that provider: it is covered by any open episode, and a success without identity ends all of that provider's episodes, so an identity gap neither re-alerts every refresh nor hides a real recovery.
  • Providers that are no longer enabled retire their episodes on the next refresh.
  • The master "Show Notifications" switch also silences these alerts, like every other toast (Windows-side choice; upstream has no master switch).

Upstream reference

  • Release: CodexBar v0.67.0, credential-expiry notifications (opt-in, account-scoped episodes).
  • Tag-pinned sources (ref=v0.67.0): Sources/CodexBar/ProviderCredentialFailure.swift, Sources/CodexBar/UsageStore+CredentialNotifications.swift (handleCredentialOutcome), docs/credential-notifications.md, AppNotifications.swift, PreferencesNotificationsPane.swift, tests CredentialNotificationTests.swift.

Ported / Deferred

Ported: episode model, per-account scoping, fresh-success-only recovery, retry after failed delivery, toggle semantics, disabled-provider retirement, generic toast copy, Settings toggle, locale keys (en-US; other locales fall back).

Deferred / not applicable:

  • Upstream's native-error classifier (ProviderCredentialFailure.isAuthenticationFailure, per-provider Swift error types) is replaced by the existing typed ProviderStateKind, per the audit spec. Classification quality therefore follows each provider's error_state_kind.
  • Claude credential-file fingerprint scope binding and the Codex visible-account owner key: no local counterpart; the existing warning identity is used.
  • Augment keepalive routing: the keepalive is not wired locally, nothing to route.
  • Withdrawing delivered toasts on recovery/disable/shutdown (AppNotifications.remove): Windows toasts here are fire-and-forget PowerShell dispatches with no handle to withdraw.
  • Post-authorization recheck of consent and episode validity: delivery is synchronous under the same lock, so there is no authorization race.
  • Codex per-account lane refreshes (refresh_codex_account_lanes) are not observed; only the primary provider refresh is.

Validation

  • cargo +1.98.0 fmt --all -- --check: clean.
  • cargo +1.98.0 clippy --workspace --all-targets -- -D warnings: clean.
  • cargo +1.98.0 test -p codexbar -- --test-threads=4: 2172 passed, 0 failed, 1 ignored (includes 11 new notifications::credential tests: per-ProviderError-variant alert table, provider-specific offline state, toast copy, fail/fail/timeout/replay/fresh-success/fail = two toasts, failed-toast retry, account/provider independence, identity-gap wildcard, toggle off then on, recovery while off, disabled-provider retirement, master-switch policy; plus needs_sign_in and settings default/back-compat assertions).
  • cargo +1.98.0 test -p codexbar-desktop-tauri -- --test-threads=4: 464 passed, 1 failed. The failure is commands::tests::bootstrap_payload_exposes_every_provider_variant (catalog 79 vs 78): it reads this machine's real settings, which have the deprecated kimik2 provider enabled, and is unrelated to this change (the same failure is reported on other port PRs). New commands::credential_alerts tests pass.
  • pnpm --dir apps/desktop-tauri exec vitest run src: 67 files, 403 passed (new toggle test in GeneralTab.test.tsx).
  • pnpm --dir apps/desktop-tauri run lint: only existing warnings in untouched files. pnpm --dir apps/desktop-tauri run build (includes check-locale): OK.
  • The toast itself was not shown on a live desktop; delivery is exercised through an injected sender in tests.

Affected areas

  • Settings UI
  • Config file / settings persistence
  • Provider-specific behavior (refresh outcome handling, no provider module changes)
  • Startup / background behavior (refresh loop notification path)
  • Tray panel / CLI / Installer / Documentation

UI proof

Pending: coordinator will capture CUA proof (Notifications tab toggle) and a manual toast capture on a fresh build.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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

Finesssee commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator Author

CUA proof for the Credential Expiry Alerts settings toggle (local Windows debug build, cua-driver CLI).

Build: commit 7cddcacd (PR head, detached). pnpm --dir apps/desktop-tauri run tauri:build:debug finished with exit 0. The launched exe was codexbar-desktop-tauri.exe.

Isolation: the exe ran with CODEXBAR_PROOF_MODE=settings:notifications, an empty CODEX_HOME, and a settings root redirected to a temp folder. The redirect came from a throwaway, uncommitted one-line patch to logging::config_root that read CODEXBAR_PROOF_CONFIG_ROOT. I reverted the patch after the build, and the worktree is clean. The user's real settings were not touched. No provider data was seeded, since this PR only adds a settings toggle.

Drive: cua-driver serve, then call list_windows, get_window_state and click (UIA Toggle on the element).

# Assertion Result
1 Notifications tab shows a "Credential Expiry Alerts" checkbox with helper text "Alert once when a provider account needs to sign in again" PASS
2 Default is off (selected: false) on a fresh config PASS
3 After a UIA toggle it is on (selected: true) PASS
4 A second toggle turns it off (selected: false) PASS
5 After toggling on and restarting the app, it is still on (persisted in settings) PASS
6 Theme is dark under the default auto setting PASS (screenshot)
7 A final toggle after restart works (off again) PASS

Not covered: toast delivery on a real sign-in failure. That is covered by the 11 new notifications::credential unit tests, not by this UI proof.

Local artifacts (not committed): %LOCALAPPDATA%\Temp\port-audit\proof\660\

  • 01-before-uia.txt (default state)
  • 02-after-on-uia.txt
  • 03-after-off-uia.txt
  • 04-after-restart-uia.txt (persistence after restart)
  • 04-after-restart-on.png (screenshot, checkbox on, dark theme)
  • 05-toggled-off-again.png

The screenshots show settings chrome only, with no emails or tokens.

The .png files from the first three steps were not written because of a path-quoting bug in my helper script. The UIA JSON dumps for those steps are intact and carry the assertions.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Reviewed by Codex gpt-6-luna (xhigh); verified and validated by Claude

Thermo-nuclear review of #660 (credential expiry alerts). Findings:

  • Medium, commands/codex_accounts.rs: managed Codex account lane failures were discarded, so those accounts could never raise a sign-in alert. Fixed: lane outcomes now feed the account-scoped failure/recovery episodes (typed CodexApiError::state_kind(); 401 = expired session, 403 = permission denied and never alerts).
  • Medium, commands/providers.rs: the alert preference was captured before the fetch, so a toggle change during an in-flight request was ignored. Fixed: consent is read at outcome time.
  • Medium, commands/providers.rs: episodes were not retired when no providers were enabled because refresh returned early. Fixed, with a regression test.
  • Low, locales: the new strings existed only in en-US. Fixed: translated in all 8 catalogs, plus a test asserting the keys exist in every catalog.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Follow-up: the four findings above are fixed and pushed. Nothing left open.

Note: a 403 from the Codex usage API now returns its own "forbidden" message (previously it shared the 401 text), so it never triggers the sign-in alert or the token-refresh retry.

Commands run (Rust 1.98.0, E-cores): cargo fmt --all; clippy --all-targets -D warnings on rust and apps/desktop-tauri/src-tauri (clean); cargo test for codex_accounts, notifications, locale (104 passed) and the shell commands:: tests (180 passed). One shell test, bootstrap_payload_exposes_every_provider_variant (catalog 79 vs 78), fails; this PR does not touch provider ids or the catalog, so it comes from the base branch. Not run: CUA (no fresh UI proof; the existing proof covers the settings toggle, which these fixes do not change).

@Finesssee

Finesssee commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

CUA proof

Build commit: 5d03b7f (PR head, detached), pnpm tauri:build:debug.

Proof-only patch (uncommitted, reverted afterwards, never pushed): root Cargo.toml [patch.crates-io] dirs = <local shim> so config/data/home dirs resolve under an isolated CODEXBAR_PROOF_HOME. No provider enabled, no network data, and the user's real settings/credentials were not touched. Patch saved at %LOCALAPPDATA%\Win-CodexBar\port-audit\proof\660\proof-only.patch.

Commands: bash launch.sh settings:general (proof mode, theme auto), then cua-driver CLI only (get_window_state, background UIA click on element tokens, window screenshots). No foreground input, no focus change; window kept on the second monitor.

# Assertion Result
0 No real email/account from the user's machine visible PASS
1 Notifications tab shows "Credential Expiry Alerts" with helper "Alert once when a provider account needs to sign in again" PASS
2 Default is unchecked with a settings.json lacking the key PASS
3 Toggle on via UIA: checkbox selected, decrypted isolated settings.json has credential_expiry_notifications_enabled = true PASS
4 Toggle off: checkbox cleared, settings.json has credential_expiry_notifications_enabled = false PASS
5 Theme auto, webview stays dark PASS

Not covered: the OS toast itself (needs a real sign-in-required provider failure); dedupe logic is covered by the PR's cargo tests.

Screenshots (local, not committed), %LOCALAPPDATA%\Win-CodexBar\port-audit\proof\660\shots\: 01-general-default.png, 02-notifications-default.png, 03-toggled-on.png, 04-toggled-off.png.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Adversarial validation (lane-B review)

Head validated: 5d03b7fb ("Address thermo review"). Spec: 0.67.0.md PR 7 (item #5, credential-expiry alerts).

Verdict: no blocking defects. Spec coverage verified in source at head:

  • Only typed sign-in states trigger alerts: ProviderStateKind::needs_sign_in() gates on NeedsAuthentication | ExpiredSession exactly — quota, permission, rate-limit and transport failures never qualify (test only_authentication_states_need_sign_in).
  • One alert per (provider, account) failure episode, in-memory; only a fresh successful fetch ends an episode; cached/degraded results do not restart or extend one (credential_alerts.rs: the fresh fetch's error state is captured before any last-good preservation — "captured from the fresh fetch before any last-good" documented + tested).
  • Notification text is only " needs sign-in" (CredentialExpiryTitle) with a generic body — no credential material echoed.
  • Toggle off suppresses delivery without forgetting episodes (settings flow through the existing notification gates).

Validation re-run at head (pinned 1.98.0, E-cores): cargo +1.98.0 fmt --all -- --check clean; cargo +1.98.0 clippy --workspace --all-targets -- -D warnings clean; cargo +1.98.0 test -p codexbar --lib 2174 passed / 0 failed (1 ignored; focused credential 60/60); cargo +1.98.0 test -p codexbar-desktop-tauri 464 passed / 1 failed (the documented #684 bootstrap_payload_exposes_every_provider_variant baseline); frontend pnpm test 67 files / 403 passed; pnpm run check-locale OK (883 keys); pnpm run build OK.

UI proof: waived per user 2026-10-02 directive (fast-track); the episode state machine is covered by the new Rust tests; the Settings toggle is covered by GeneralTab tests.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

UI proof (browser-use)

Combined build of integrate/v0.70.0-ports at ddd85594 (all 40 port PRs and the follow-ups). Debug desktop build in proof mode, isolated config/data dirs, driven by browser-use over the WebView2 DevTools protocol with DOM events only (no mouse or keyboard input, no focus changes). All data is synthetic: local mock servers for L1, a seeded usage snapshot plus synthetic local logs for L2.

Scenario Check Result
L1 extras (preferences, keys, toggles) #660 Notifications: 'Credential Expiry Alerts' toggle turns on and off and persists PASS

Every surface also passed the privacy check (no email-like text, account e-mail nodes or profile paths in the DOM) and theme auto rendered dark on the tray flyout, float bar and settings windows.

Validation at ddd85594: cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test -p codexbar (3567 passed, 0 failed) and cargo test -p codexbar-desktop-tauri (606 passed, 0 failed). Screenshots were captured for each scenario and kept with the local proof kit.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Shipped in v0.70.0: this PR's head is included in main via #735 (merge commit 9d0a37a). Closing as integrated.

@Finesssee Finesssee closed this Oct 3, 2026
junglesub-bot Bot pushed a commit to junglesub/Win-CodexBar that referenced this pull request Oct 4, 2026
Conflicts: rust/src/notifications.rs keeps the release identity_gaps
module and test-only toast capture next to the new credential episodes.
The PR makes show_toast return whether the toast was handed to the OS;
the release's #[cfg(test)] recorder now returns true (recorded = handed
over), and the Windows / non-Windows senders stay #[cfg(not(test))], so
credential tests never spawn PowerShell. locale/tests.rs keeps both the
Aixy gateway and credential-expiry key checks. PopOutPanel.test.tsx
stays deleted (PopOut layout retired on the release).
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