Skip to content

[codex] Structure desktop persisted credential errors#3239

Open
juliusmarminge wants to merge 1 commit into
mainfrom
codex/desktop-structured-errors-persisted-credentials
Open

[codex] Structure desktop persisted credential errors#3239
juliusmarminge wants to merge 1 commit into
mainfrom
codex/desktop-structured-errors-persisted-credentials

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Desktop connection catalogs and saved-environment credentials crossed the Electron safe-storage boundary without adding any domain context. When availability checks, encryption, or decryption failed, telemetry only received an ElectronSafeStorage*Error; it could not identify the affected catalog path, saved-environment ID, or operation. The original cause chain was present, but it was not enough to correlate a failure with persisted state.

This change introduces domain errors for those boundaries. Catalog failures now carry the catalog path and one of the availability/encrypt/decrypt operations. Saved-environment secret failures carry the registry path, environment ID, and secret-protection operation. Each wrapper retains the complete upstream ElectronSafeStorage*Error as cause, so the underlying defect and stack remain available while messages are derived only from stable structured fields.

The refactor also removes the valueless writeError and migrationError constructor aliases in both modules. Construction now happens at each failure site, making the operation, path, environment ID, and preserved cause visible without following an indirection.

Validation:

  • vp test apps/desktop/src/app/DesktopConnectionCatalogStore.test.ts apps/desktop/src/settings/DesktopSavedEnvironments.test.ts (23 tests)
  • vp check (passes; existing unrelated warnings remain)
  • vp run typecheck

Focused tests assert the new correlation fields, stable messages, and both levels of each preserved safe-storage cause chain.


Note

Medium Risk
Changes error types and wrapping around Electron safe storage for persisted credentials; callers and telemetry must handle the new tagged errors, though behavior for unavailable encryption (false/none) is largely unchanged.

Overview
Desktop connection catalog and saved-environment flows no longer surface raw ElectronSafeStorage* failures at their public API. Each encrypt/decrypt/availability failure is wrapped in a domain tagged error that carries catalog path or registry path + environment ID, plus a stable operation label, while keeping the full upstream safe-storage error in cause.

DesktopConnectionCatalogStoreProtectionError replaces direct safe-storage errors on get/set. DesktopSavedEnvironmentSecretProtectionError does the same for secret read/write paths, including availability checks that now fail with context instead of only returning false when the availability probe errors.

The writeError / migrationError helpers are removed in favor of constructing write/migration errors inline at each failure site. Tests are updated to assert wrapper fields, messages, and preserved cause chains.

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

Note

Structure persisted credential errors in desktop safe storage into typed error classes

  • Introduces DesktopConnectionCatalogStoreProtectionError and DesktopSavedEnvironmentSecretProtectionError as typed error classes, replacing raw ElectronSafeStorage errors surfaced to callers of get/set on the connection catalog store and getSecret/setSecret on saved environments.
  • Each error carries a structured operation field (check-encryption-availability, encrypt-*, decrypt-*), the relevant path/ID context, and the original error as a typed cause.
  • Error mapping is applied consistently at the call sites for isEncryptionAvailable, encryptString, and decryptString in both DesktopConnectionCatalogStore.ts and DesktopSavedEnvironments.ts.
  • Behavioral Change: callers that previously caught ElectronSafeStorageError or ElectronSafeStorageAvailabilityError directly must now handle the new typed protection errors.

Macroscope summarized 9de41e1.

Co-authored-by: codex <codex@users.noreply.github.com>
@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: 446ec4f8-c56f-40b8-ba82-45cf802c04a1

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-structured-errors-persisted-credentials

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

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This PR restructures error handling by introducing domain-specific error classes that wrap underlying storage errors with additional context (operation type, paths). The success-path behavior is unchanged; only error representation is modified, with corresponding test updates.

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

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