Skip to content

Verifiable reset notifications + Codex banked-resets 0-state#43

Closed
tsouth89 wants to merge 2 commits into
mainfrom
feat/reset-notif-and-codex-banked-resets
Closed

Verifiable reset notifications + Codex banked-resets 0-state#43
tsouth89 wants to merge 2 commits into
mainfrom
feat/reset-notif-and-codex-banked-resets

Conversation

@tsouth89

@tsouth89 tsouth89 commented Jul 17, 2026

Copy link
Copy Markdown
Owner

First pass of a multi-pass reliability/UX effort on Ceiling. Two focused, independently-reviewable changes.

1. Show Codex banked resets, including an explicit "0" state

Codex is the only provider with banked rate-limit resets, but the indicator was invisible whenever the count was zero (backend collapsed 0 to None; surfaces only rendered when non-null). A user with no banked resets couldn't tell the feature was being tracked.

  • codex/api.rs: record Some(available_count) including 0 on a successful fetch. None now means only "not fetched / unknown".
  • capacityPresentation: resetCreditsAvailable returns 0 as a real value; new codexResetCredits() keeps it Codex-only.
  • PlanStatusCard, ProviderDetailView, TaskbarFlyout: always show the chip for Codex, with a muted --empty variant at 0.

2. "Send test notification" button to verify toast delivery

Unexpected-reset toasts are already wired and default-on, but there was no way to confirm the Windows toast pipeline (AUMID + PowerShell + WinRT) actually delivers on a given machine.

  • notifications.rs: extracted the shared toast-script builder; added send_test_notification() that runs the real pipeline synchronously and returns Ok/Err. It intentionally bypasses the startup gate and rate limits — it's user-initiated and must prove delivery end-to-end.
  • Tauri command (registered + allowlisted), sendTestNotification() bridge, and a Settings button with sending/sent/failed states.
  • New NotificationTest* locale keys across locale.rs, keys.ts, and all 6 .ftl files.

Verification

  • Rust codexbar lib: 630 passed (incl. new zero-count test)
  • Frontend: 224 passed (incl. new zero-count, Codex-only, and test-button tests); tsc clean; locale-drift OK
  • Desktop crate: cargo check clean

Next pass

SOU-122 — explicit enforcement states (tracked / not-enforced / unavailable) without fabricating a %. Kept separate for reviewability.

Summary by CodeRabbit

  • New Features

    • Added a “Send test notification” option in notification settings, with sending, success, and failure states.
    • Added localized notification-test messaging across supported languages.
    • Improved notification delivery testing across Windows and other supported platforms.
  • Bug Fixes

    • Reset-credit counts now display correctly when the available value is zero.
    • Reset-credit indicators are shown only for applicable providers and use a subdued empty state.
  • Tests

    • Added coverage for notification testing and zero reset-credit handling.

tsouth89 added 2 commits July 17, 2026 19:52
Codex is the only provider with banked rate-limit resets, and the
indicator was hidden whenever the count was zero: the backend collapsed
a zero count to None and every surface only rendered the chip when the
value was non-null. A user with no banked resets saw nothing and had no
way to know the feature was being tracked.

- codex/api.rs: record Some(available_count) including 0 on a successful
  fetch. None now means only "not fetched / unknown", never "zero".
- capacityPresentation: resetCreditsAvailable returns 0 as a real value;
  new codexResetCredits() keeps the indicator Codex-only.
- PlanStatusCard, ProviderDetailView, TaskbarFlyout: always show the chip
  for Codex, with a muted --empty variant when the count is 0.
Unexpected-reset toasts are already wired and default-on, but there was
no way to confirm the Windows toast pipeline (AUMID registration +
PowerShell + WinRT) actually delivers on a given machine, and debug
builds suppress toasts entirely.

- notifications.rs: extract the shared toast-script builder and add
  send_test_notification(), which runs the real pipeline synchronously
  (.output()) and returns Ok/Err. It deliberately bypasses the startup
  baseline gate and rate limits: it is user-initiated and must prove
  delivery end-to-end.
- Expose it as a Tauri command (registered + allowlisted) and a
  sendTestNotification() bridge wrapper.
- Settings > Notifications: a "Send test" button with sending/sent/failed
  states, disabled when notifications are off.
- New NotificationTest* locale keys across locale.rs, keys.ts, and all 6
  .ftl files (English).
@cloudflare-workers-and-pages

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
ceiling f17c392 Commit Preview URL

Branch Preview URL
Jul 17 2026, 11:54 PM

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a settings control for sending test notifications through Tauri and native notification APIs. It also preserves explicit zero Codex reset credits and renders them as muted indicators across capacity-related views.

Changes

Notification testing and reset-credit presentation

Layer / File(s) Summary
Preserve and expose zero reset credits
rust/src/providers/codex/api.rs, apps/desktop-tauri/src/lib/capacityPresentation.*
Codex usage now records Some(0) reset credits, while presentation helpers restrict reset-credit display to Codex and validate zero-value behavior.
Render zero reset-credit states
apps/desktop-tauri/src/components/PlanStatusCard.tsx, apps/desktop-tauri/src/surfaces/{ProviderDetailView,TaskbarFlyout}.tsx, apps/desktop-tauri/src/styles.css
Capacity views apply empty-state classes and muted styling when reset credits equal zero.
Implement native test notification dispatch
rust/src/notifications.rs
Toast construction is extracted into helpers, and synchronous Windows plus non-Windows notification test dispatch returns success or diagnostic errors.
Wire notification testing into settings
apps/desktop-tauri/src/surfaces/settings/tabs/GeneralTab.*, apps/desktop-tauri/src/lib/tauri.ts, apps/desktop-tauri/src-tauri/*, rust/src/locale*, apps/desktop-tauri/src/i18n/keys.ts
The command is permitted and registered, exposed through the frontend bridge, rendered with sending/sent/failed states, localized, and covered by settings tests.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GeneralTab
  participant TauriBridge
  participant NotificationCommand
  participant notifications
  GeneralTab->>TauriBridge: sendTestNotification()
  TauriBridge->>NotificationCommand: invoke send_test_notification
  NotificationCommand->>notifications: send_test_notification()
  notifications-->>NotificationCommand: Result<(), String>
  NotificationCommand-->>TauriBridge: command result
  TauriBridge-->>GeneralTab: success or failure
Loading

Possibly related PRs

Suggested reviewers: finesssee

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately reflects the two main changes: test notifications and Codex reset-credit zero-state handling.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/reset-notif-and-codex-banked-resets
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feat/reset-notif-and-codex-banked-resets

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

@tsouth89

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
rust/src/locale/ja-JP.ftl (1)

1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

New NotificationTest* locale entries are untranslated across all four non-English locale files. The shared root cause is that all six new notification-test strings were added in English instead of being translated to each locale's language.

  • rust/src/locale/ja-JP.ftl#L369-374: Translate NotificationTest, NotificationTestHelper, NotificationTestButton, NotificationTestSending, NotificationTestSent, and NotificationTestFailed to Japanese.
  • rust/src/locale/ko-KR.ftl#L369-374: Translate the same six keys to Korean.
  • rust/src/locale/zh-CN.ftl#L369-374: Translate the same six keys to Simplified Chinese.
  • rust/src/locale/zh-TW.ftl#L369-374: Translate the same six keys to Traditional Chinese.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/locale/ja-JP.ftl` at line 1, Translate the six NotificationTest
locale entries—NotificationTest, NotificationTestHelper, NotificationTestButton,
NotificationTestSending, NotificationTestSent, and NotificationTestFailed—in
each non-English locale file: Japanese, Korean, Simplified Chinese, and
Traditional Chinese. Preserve the existing keys and provide natural translations
matching each locale’s language.
🧹 Nitpick comments (1)
apps/desktop-tauri/src/surfaces/settings/tabs/GeneralTab.test.tsx (1)

118-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding a test for the "failed" notification state.

The success and disabled paths are covered, but the failure path (sendTestNotification rejects → button shows NotificationTestFailed) is untested.

✨ Suggested test
it("shows failure message when notification send fails", async () => {
  sendTestNotificationMock.mockRejectedValueOnce(new Error("fail"));
  render(
    <GeneralTab
      mode="notifications"
      settings={settings}
      set={vi.fn()}
      saving={false}
    />,
  );

  fireEvent.click(screen.getByRole("button", { name: "NotificationTestButton" }));

  await waitFor(() =>
    expect(
      screen.getByRole("button", { name: "NotificationTestFailed" }),
    ).toBeInTheDocument(),
  );
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop-tauri/src/surfaces/settings/tabs/GeneralTab.test.tsx` around
lines 118 - 152, Add a failure-path test alongside the existing notification
tests, using sendTestNotificationMock to reject once, rendering GeneralTab with
notifications enabled, clicking the NotificationTestButton, and awaiting the
NotificationTestFailed button state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rust/src/locale/es-MX.ftl`:
- Around line 369-374: Translate the six notification-test localization
values—NotificationTest, NotificationTestHelper, NotificationTestButton,
NotificationTestSending, NotificationTestSent, and NotificationTestFailed—into
natural Spanish suitable for the Windows settings UI, preserving their existing
Fluent keys and meaning.

In `@rust/src/locale/ja-JP.ftl`:
- Around line 369-374: Translate all six NotificationTest* entries in the
Japanese locale, including the title, helper text, button label, sending and
sent statuses, and failure message. Keep the existing Fluent keys unchanged and
provide natural Japanese user-facing text.

In `@rust/src/locale/ko-KR.ftl`:
- Around line 369-374: Translate all six NotificationTest* entries in the Korean
locale, including the title, helper text, button label, sending and sent
statuses, and failure message, while preserving each existing Fluent key and
placeholder-free message structure.

In `@rust/src/locale/zh-CN.ftl`:
- Around line 369-374: Translate all six NotificationTest,
NotificationTestHelper, NotificationTestButton, NotificationTestSending,
NotificationTestSent, and NotificationTestFailed values into Simplified Chinese
while preserving their existing Fluent keys and notification meanings.

In `@rust/src/locale/zh-TW.ftl`:
- Around line 369-374: Translate all six NotificationTest*,
NotificationTestHelper, NotificationTestButton, NotificationTestSending,
NotificationTestSent, and NotificationTestFailed values into natural Traditional
Chinese while preserving their keys and Fluent syntax.

In `@rust/src/notifications.rs`:
- Around line 763-768: Update ensure_aumid_registered_once to return a Result
and cache completion only after successful AUMID registration, allowing later
calls to retry after failure. Propagate the registration error through
send_test_notification so the test stops before invoking PowerShell, while
preserving clear, user-facing diagnostics with friendly error messages.
- Around line 788-798: Bound both the PowerShell and notify-send notifier
processes with a finite deadline instead of awaiting Command::output()
indefinitely. Update the notifier execution around Command::new("powershell")
and its notify-send counterpart to monitor the child, terminate it when the
deadline expires, and return a clear user-friendly timeout error while
preserving existing launch-error handling.

---

Outside diff comments:
In `@rust/src/locale/ja-JP.ftl`:
- Line 1: Translate the six NotificationTest locale entries—NotificationTest,
NotificationTestHelper, NotificationTestButton, NotificationTestSending,
NotificationTestSent, and NotificationTestFailed—in each non-English locale
file: Japanese, Korean, Simplified Chinese, and Traditional Chinese. Preserve
the existing keys and provide natural translations matching each locale’s
language.

---

Nitpick comments:
In `@apps/desktop-tauri/src/surfaces/settings/tabs/GeneralTab.test.tsx`:
- Around line 118-152: Add a failure-path test alongside the existing
notification tests, using sendTestNotificationMock to reject once, rendering
GeneralTab with notifications enabled, clicking the NotificationTestButton, and
awaiting the NotificationTestFailed button state.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4d398440-6832-46f8-a604-688d3c352e5a

📥 Commits

Reviewing files that changed from the base of the PR and between 6b8ff38 and f17c392.

📒 Files selected for processing (22)
  • apps/desktop-tauri/src-tauri/permissions/commands.toml
  • apps/desktop-tauri/src-tauri/src/commands/system.rs
  • apps/desktop-tauri/src-tauri/src/main.rs
  • apps/desktop-tauri/src/components/PlanStatusCard.tsx
  • apps/desktop-tauri/src/i18n/keys.ts
  • apps/desktop-tauri/src/lib/capacityPresentation.test.ts
  • apps/desktop-tauri/src/lib/capacityPresentation.ts
  • apps/desktop-tauri/src/lib/tauri.ts
  • apps/desktop-tauri/src/styles.css
  • apps/desktop-tauri/src/surfaces/ProviderDetailView.tsx
  • apps/desktop-tauri/src/surfaces/TaskbarFlyout.tsx
  • apps/desktop-tauri/src/surfaces/settings/tabs/GeneralTab.test.tsx
  • apps/desktop-tauri/src/surfaces/settings/tabs/GeneralTab.tsx
  • rust/src/locale.rs
  • rust/src/locale/en-US.ftl
  • rust/src/locale/es-MX.ftl
  • rust/src/locale/ja-JP.ftl
  • rust/src/locale/ko-KR.ftl
  • rust/src/locale/zh-CN.ftl
  • rust/src/locale/zh-TW.ftl
  • rust/src/notifications.rs
  • rust/src/providers/codex/api.rs

Comment thread rust/src/locale/es-MX.ftl
Comment on lines +369 to +374
NotificationTest = Test notifications
NotificationTestHelper = Send a sample alert to confirm Windows delivers Ceiling notifications
NotificationTestButton = Send test
NotificationTestSending = Sending…
NotificationTestSent = Sent
NotificationTestFailed = Couldn't send. Check Windows notification settings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Translate the notification-test flow into Spanish.

These new values are English, so the Spanish settings UI becomes partially untranslated.

Proposed translations
-NotificationTest = Test notifications
-NotificationTestHelper = Send a sample alert to confirm Windows delivers Ceiling notifications
-NotificationTestButton = Send test
-NotificationTestSending = Sending…
-NotificationTestSent = Sent
-NotificationTestFailed = Couldn't send. Check Windows notification settings.
+NotificationTest = Probar notificaciones
+NotificationTestHelper = Envía una alerta de muestra para confirmar que Windows entrega las notificaciones de Ceiling
+NotificationTestButton = Enviar prueba
+NotificationTestSending = Enviando…
+NotificationTestSent = Enviada
+NotificationTestFailed = No se pudo enviar. Revisa la configuración de notificaciones de Windows.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
NotificationTest = Test notifications
NotificationTestHelper = Send a sample alert to confirm Windows delivers Ceiling notifications
NotificationTestButton = Send test
NotificationTestSending = Sending
NotificationTestSent = Sent
NotificationTestFailed = Couldn't send. Check Windows notification settings.
NotificationTest = Probar notificaciones
NotificationTestHelper = Envía una alerta de muestra para confirmar que Windows entrega las notificaciones de Ceiling
NotificationTestButton = Enviar prueba
NotificationTestSending = Enviando
NotificationTestSent = Enviada
NotificationTestFailed = No se pudo enviar. Revisa la configuración de notificaciones de Windows.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/locale/es-MX.ftl` around lines 369 - 374, Translate the six
notification-test localization values—NotificationTest, NotificationTestHelper,
NotificationTestButton, NotificationTestSending, NotificationTestSent, and
NotificationTestFailed—into natural Spanish suitable for the Windows settings
UI, preserving their existing Fluent keys and meaning.

Comment thread rust/src/locale/ja-JP.ftl
Comment on lines +369 to +374
NotificationTest = Test notifications
NotificationTestHelper = Send a sample alert to confirm Windows delivers Ceiling notifications
NotificationTestButton = Send test
NotificationTestSending = Sending…
NotificationTestSent = Sent
NotificationTestFailed = Couldn't send. Check Windows notification settings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

New NotificationTest* entries are untranslated in this Japanese locale file.

All six new notification test strings (lines 369–374) are in English. User-facing labels like "Send test" and "Sending…" should be translated to Japanese for a proper localized experience.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/locale/ja-JP.ftl` around lines 369 - 374, Translate all six
NotificationTest* entries in the Japanese locale, including the title, helper
text, button label, sending and sent statuses, and failure message. Keep the
existing Fluent keys unchanged and provide natural Japanese user-facing text.

Comment thread rust/src/locale/ko-KR.ftl
Comment on lines +369 to +374
NotificationTest = Test notifications
NotificationTestHelper = Send a sample alert to confirm Windows delivers Ceiling notifications
NotificationTestButton = Send test
NotificationTestSending = Sending…
NotificationTestSent = Sent
NotificationTestFailed = Couldn't send. Check Windows notification settings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

New NotificationTest* entries are untranslated in this Korean locale file.

All six new notification test strings (lines 369–374) are in English. User-facing labels like "Send test" and "Sending…" should be translated to Korean for a proper localized experience.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/locale/ko-KR.ftl` around lines 369 - 374, Translate all six
NotificationTest* entries in the Korean locale, including the title, helper
text, button label, sending and sent statuses, and failure message, while
preserving each existing Fluent key and placeholder-free message structure.

Comment thread rust/src/locale/zh-CN.ftl
Comment on lines +369 to +374
NotificationTest = Test notifications
NotificationTestHelper = Send a sample alert to confirm Windows delivers Ceiling notifications
NotificationTestButton = Send test
NotificationTestSending = Sending…
NotificationTestSent = Sent
NotificationTestFailed = Couldn't send. Check Windows notification settings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

New NotificationTest* entries are untranslated in this Simplified Chinese locale file.

All six new notification test strings (lines 369–374) are in English. User-facing labels like "Send test" and "Sending…" should be translated to Simplified Chinese for a proper localized experience.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/locale/zh-CN.ftl` around lines 369 - 374, Translate all six
NotificationTest, NotificationTestHelper, NotificationTestButton,
NotificationTestSending, NotificationTestSent, and NotificationTestFailed values
into Simplified Chinese while preserving their existing Fluent keys and
notification meanings.

Comment thread rust/src/locale/zh-TW.ftl
Comment on lines +369 to +374
NotificationTest = Test notifications
NotificationTestHelper = Send a sample alert to confirm Windows delivers Ceiling notifications
NotificationTestButton = Send test
NotificationTestSending = Sending…
NotificationTestSent = Sent
NotificationTestFailed = Couldn't send. Check Windows notification settings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

New NotificationTest* entries are untranslated in this Traditional Chinese locale file.

All six new notification test strings (lines 369–374) are in English. User-facing labels like "Send test" and "Sending…" should be translated to Traditional Chinese for a proper localized experience.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/locale/zh-TW.ftl` around lines 369 - 374, Translate all six
NotificationTest*, NotificationTestHelper, NotificationTestButton,
NotificationTestSending, NotificationTestSent, and NotificationTestFailed values
into natural Traditional Chinese while preserving their keys and Fluent syntax.

Comment thread rust/src/notifications.rs
Comment on lines +763 to +768
/// Register the AUMID at most once per process before showing a toast.
#[cfg(target_os = "windows")]
fn ensure_aumid_registered_once() {
use std::sync::Once;
static AUMID_INIT: Once = Once::new();
AUMID_INIT.call_once(ensure_aumid_registered);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Propagate AUMID registration failures and allow retries.

Once::call_once is permanently completed even when ensure_aumid_registered only logs a registry error. The test then continues through PowerShell, and subsequent button clicks cannot retry registration.

Return a Result and cache only successful registration; propagate failures from send_test_notification.

As per coding guidelines, preserve clear error handling and user-facing diagnostics using friendly messages where applicable.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/notifications.rs` around lines 763 - 768, Update
ensure_aumid_registered_once to return a Result and cache completion only after
successful AUMID registration, allowing later calls to retry after failure.
Propagate the registration error through send_test_notification so the test
stops before invoking PowerShell, while preserving clear, user-facing
diagnostics with friendly error messages.

Source: Coding guidelines

Comment thread rust/src/notifications.rs
Comment on lines +788 to +798
let output = Command::new("powershell")
.args([
"-NoProfile",
"-ExecutionPolicy",
"Bypass",
"-Command",
&toast_powershell_script(title, body),
])
.creation_flags(0x08000000) // CREATE_NO_WINDOW
.output()
.map_err(|e| format!("Could not launch the Windows notifier: {e}"))?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Bound synchronous notifier execution with a timeout.

Both powershell and notify-send can remain running indefinitely. Because the Tauri request awaits Command::output(), Settings remains stuck in its sending state with no recovery.

Apply a deadline, terminate the child on expiry, and return a friendly timeout error.

Also applies to: 823-825

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/notifications.rs` around lines 788 - 798, Bound both the PowerShell
and notify-send notifier processes with a finite deadline instead of awaiting
Command::output() indefinitely. Update the notifier execution around
Command::new("powershell") and its notify-send counterpart to monitor the child,
terminate it when the deadline expires, and return a clear user-friendly timeout
error while preserving existing launch-error handling.

tsouth89 added a commit that referenced this pull request Jul 18, 2026
…es (#46)

Collapses three reviewed passes onto current `main` (1.0.0-rc.1) as one
clean merge, per the plan. Supersedes #43, #44, #45 (each fully reviewed
by adversarial subagents + CodeRabbit; all findings addressed). Only
conflict was the English-only `.ftl` removals, resolved by accepting the
deletion.

## What's included
- **Codex banked resets (#43):** explicit "0 avail" chip (Codex-only,
all 3 surfaces); backend records `Some(0)` instead of hiding it. **Send
test notification** button that runs the real Windows toast pipeline and
reports success/failure.
- **Explicit enforcement states — SOU-122 (#44):** `EnforcementState {
Tracked, NotEnforced, Unavailable }` exposed to every surface without
fabricating a %. A process-local, per-scope `EnforcementTracker` flags a
core subscription window `unavailable` when it drops out of a successful
response — sticky until it returns, core-windows-only (bonus pools never
false-flag), scoped by provider|source|email|org, ignores errors.
- **Exact & Calm floatbar modes — SOU-178 (#45):**
`float_bar_information_mode` (Exact default, safe migration). Calm leads
with a trustworthy pace state + next reset; exact % on keyboard/click
expand. Pace claimed only when live and actually supported.

## Verification (against this main)
- Rust `codexbar` lib **621**, desktop crate **372** — 0 failures
- Frontend **246** (48 files); `tsc` clean; locale-drift OK
(English-only)

## Review fixes folded in
Sticky/core-only tracker (one-shot bug), org-scoped observations,
compact-density calm layout, calm aria-label matching visible state.

---------

Co-authored-by: tsouth89 <tsouth89@users.noreply.github.com>
@tsouth89

Copy link
Copy Markdown
Owner Author

Superseded by #46, which collapsed all three reviewed passes onto current main (1.0.0-rc.1) and merged. All findings from this PR's reviews were addressed there.

@tsouth89 tsouth89 closed this Jul 18, 2026
@tsouth89
tsouth89 deleted the feat/reset-notif-and-codex-banked-resets branch July 18, 2026 01:45
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