Skip to content

[codex] add structured Electron dialog errors - #3284

Merged
juliusmarminge merged 2 commits into
mainfrom
codex/desktop-electron-dialog-errors
Jun 20, 2026
Merged

[codex] add structured Electron dialog errors#3284
juliusmarminge merged 2 commits into
mainfrom
codex/desktop-electron-dialog-errors

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • model Electron dialog failures as schema-backed errors with operation-specific context
  • preserve original causes for folder picker, confirmation, message-box, and fatal error-box failures
  • add focused coverage for structured attributes, messages, predicates, and cause chains

Verification

  • vp test apps/desktop/src/electron/ElectronDialog.test.ts
  • vp check (0 errors; 20 existing warnings)
  • vp run typecheck

Sensitive-context follow-up\n- confirmation, message-box, and error-box payload text is represented only by lengths and counts\n- owner window, dialog type, and intentional folder default paths remain available for correlation\n- all original Electron causes remain intact


Note

Medium Risk
Changes failure channels for dialog APIs (typed failures vs generic rejections; error box as defects), which callers must handle correctly though happy-path behavior is unchanged.

Overview
Electron dialog failures are now typed, schema-backed errors instead of raw promise rejections.

pickFolder, confirm, and showMessageBox use Effect.tryPromise and fail with operation-specific tagged errors that keep the original cause plus safe metadata (window id, path, prompt/message lengths, button counts)—not full user-facing strings in message. An ElectronDialogError union and isElectronDialogError are exported for callers.

showErrorBox wraps sync throws in ElectronDialogShowErrorBoxError and Effect.orDie, so those failures surface as defects rather than recoverable typed errors.

New tests assert error shape, predicate usage, and that messages omit prompt/title/content and cause text.

Reviewed by Cursor Bugbot for commit 513653f. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add structured error types to ElectronDialog methods for failures and defects

  • Introduces four Schema.TaggedErrorClass types (ElectronDialogPickFolderError, ElectronDialogConfirmError, ElectronDialogShowMessageBoxError, ElectronDialogShowErrorBoxError) in ElectronDialog.ts, each carrying summarized request context (e.g. lengths, IDs) and the original cause.
  • Replaces Effect.promise/Effect.sync calls with Effect.tryPromise/Effect.try so rejections and throws are caught and wrapped in the appropriate typed error.
  • showErrorBox failures are converted to defects via Effect.orDie, matching Electron's fire-and-forget semantics.
  • Adds an ElectronDialogError union type and isElectronDialogError predicate for runtime type-checking.
  • Error messages include only summarized metadata (lengths, counts, IDs) and never expose raw prompt text, titles, or the original cause message.
  • Behavioral Change: pickFolder, confirm, and showMessageBox now fail with typed errors instead of untyped rejections; showErrorBox now dies with a typed defect instead of completing silently on throw.

Macroscope summarized 513653f.

Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge juliusmarminge added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Jun 20, 2026
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4b9ab130-7594-4b30-a202-736dc414bc82

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/desktop-electron-dialog-errors

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

@github-actions github-actions Bot added the size:L 100-499 changed lines (additions + deletions). label Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Adds structured error classes for Electron dialog operations without changing dialog behavior itself. The changes are self-contained error handling improvements with comprehensive test coverage. The unresolved comment identifies a potential edge case in error construction, not a runtime behavioral issue.

You can customize Macroscope's approvability policy. Learn more.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
@macroscopeapp
macroscopeapp Bot dismissed their stale review June 20, 2026 16:10

Dismissing prior approval to re-evaluate 513653f

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Unsafe message length in catch
    • Added optional chaining (options.message?.length ?? null) and updated the messageLength schema to Schema.NullOr(Schema.Number) to safely handle the optional message field, consistent with how titleLength and detailLength are already handled.

Create PR

Or push these changes by commenting:

@cursor push 17d1f18c8c
Preview (17d1f18c8c)
diff --git a/apps/desktop/src/electron/ElectronDialog.ts b/apps/desktop/src/electron/ElectronDialog.ts
--- a/apps/desktop/src/electron/ElectronDialog.ts
+++ b/apps/desktop/src/electron/ElectronDialog.ts
@@ -42,7 +42,7 @@
   {
     type: Schema.NullOr(Schema.Literals(["none", "info", "error", "question", "warning"])),
     titleLength: Schema.NullOr(Schema.Number),
-    messageLength: Schema.Number,
+    messageLength: Schema.NullOr(Schema.Number),
     detailLength: Schema.NullOr(Schema.Number),
     buttonCount: Schema.Number,
     cause: Schema.Defect(),
@@ -177,7 +177,7 @@
         new ElectronDialogShowMessageBoxError({
           type: options.type ?? null,
           titleLength: options.title?.length ?? null,
-          messageLength: options.message.length,
+          messageLength: options.message?.length ?? null,
           detailLength: options.detail?.length ?? null,
           buttonCount: options.buttons?.length ?? 0,
           cause,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 513653f. Configure here.

Comment thread apps/desktop/src/electron/ElectronDialog.ts
@juliusmarminge
juliusmarminge merged commit be4a38e into main Jun 20, 2026
17 checks passed
@juliusmarminge
juliusmarminge deleted the codex/desktop-electron-dialog-errors branch June 20, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant