[codex] Structure desktop persisted credential errors#3239
[codex] Structure desktop persisted credential errors#3239juliusmarminge wants to merge 1 commit into
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: 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. |
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*Errorascause, so the underlying defect and stack remain available while messages are derived only from stable structured fields.The refactor also removes the valueless
writeErrorandmigrationErrorconstructor 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 typecheckFocused 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 incause.DesktopConnectionCatalogStoreProtectionErrorreplaces direct safe-storage errors onget/set.DesktopSavedEnvironmentSecretProtectionErrordoes the same for secret read/write paths, including availability checks that now fail with context instead of only returningfalsewhen the availability probe errors.The
writeError/migrationErrorhelpers 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
DesktopConnectionCatalogStoreProtectionErrorandDesktopSavedEnvironmentSecretProtectionErroras typed error classes, replacing rawElectronSafeStorageerrors surfaced to callers ofget/seton the connection catalog store andgetSecret/setSecreton saved environments.operationfield (check-encryption-availability,encrypt-*,decrypt-*), the relevant path/ID context, and the original error as a typed cause.isEncryptionAvailable,encryptString, anddecryptStringin both DesktopConnectionCatalogStore.ts and DesktopSavedEnvironments.ts.ElectronSafeStorageErrororElectronSafeStorageAvailabilityErrordirectly must now handle the new typed protection errors.Macroscope summarized 9de41e1.