Skip to content

fix(desktop): select Windows encryption profile before startup yields - #14265

Open
widingmarcus-cyber wants to merge 1 commit into
pingdotgg:mainfrom
widingmarcus-cyber:codex/8341-select-profile-before-ready
Open

widingmarcus-cyber wants to merge 1 commit into
pingdotgg:mainfrom
widingmarcus-cyber:codex/8341-select-profile-before-ready

Conversation

@widingmarcus-cyber

@widingmarcus-cyber widingmarcus-cyber commented Sep 29, 2026 •

Copy link
Copy Markdown

What Changed

Select the existing Windows Electron userData profile synchronously in DesktopPreReadyPlatform, before asynchronous startup can yield to Electron readiness. Initialize safeStorage availability on ready with that profile selected.

Preserve the existing legacy/current and development/packaged profile choices and APPDATA/home fallback. A filesystem error inspecting the legacy profile is not treated as a missing profile. Linux and macOS behavior is unchanged.

Why

Addresses #8341; related to #13656.

A Windows Alpha 0.0.42 installation repeatedly reported decrypt-catalog failures and displayed no projects. The same encrypted catalogs were successfully decrypted using the existing Windows Local State key in an isolated Electron process. A local installed-app patch selecting the profile before asynchronous startup and initializing secure storage after ready restored the existing project without resetting the catalog, and passed two consecutive restarts.

This points to startup ordering as a contributor, but does not establish the exact intermittent trigger or explain the original crash. This PR moves that ordering into the existing pre-ready setup. It does not reset, migrate, or back up credentials. #13656 separately proposes recovery after a decryption failure; this PR is independent of it and preserves the existing catalog.

Validation

  • 20 focused tests pass across DesktopPreReadyPlatform, DesktopClerk, and DesktopAppIdentity.
  • Four new ordering cases fail against unmodified upstream: current/legacy profiles in packaged/development mode. They verify profile selection before a queued readiness turn and key initialization against that selected profile.
  • Added a profile-inspection failure case to ensure startup does not silently switch profiles on an access error.
  • Desktop TypeScript check, targeted lint, formatting, and git diff --check pass.
  • Local installed-app evidence: Windows, Alpha 0.0.42 / Electron 44.1.0; existing connections readable across two restarts and the user confirmed the project returned.
  • A full desktop package built from this PR has not been tested. Unit tests enforce ordering; they do not reproduce the intermittent native decryption failure itself.

Checklist

  • Small, focused change: two files, 19 production lines plus tests.
  • Problem, behavior, and validation boundaries explained.
  • UI screenshots/video: not applicable; no UI component or animation changes.

Prepared with GPT-6 in the Codex desktop harness.

Summary by CodeRabbit

  • Bug Fixes
    • Windows startup now uses an existing app profile when available, helping preserve access to existing app data. If no legacy profile is found, startup uses the current profile. This applies to both development and packaged versions of the app.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 29, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This Windows startup change selects the Electron encryption profile earlier and initializes safeStorage before readiness, affecting decryption of locally stored connection data and credentials. Despite its narrow scope and focused tests, the sensitive-data runtime impact warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 5f033178-6932-4384-ac81-b0bdec98062c

📥 Commits

Reviewing files that changed from the base of the PR and between d2c9281 and 6064b5f.

📒 Files selected for processing (2)
  • apps/desktop/src/app/DesktopPreReadyPlatform.test.ts
  • apps/desktop/src/app/DesktopPreReadyPlatform.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

On Windows, startup selects a legacy profile when it exists and otherwise uses the current profile. It sets Electron’s userData path before returning and checks safe-storage encryption availability when Electron is ready.

Changes

Windows startup profile selection

Layer / File(s) Summary
Select the profile and check encryption
apps/desktop/src/app/DesktopPreReadyPlatform.ts, apps/desktop/src/app/DesktopPreReadyPlatform.test.ts
On Windows, startup selects the legacy or current profile and sets Electron’s userData path. Tests cover development and packaged startup, encryption checks, and errors during legacy-profile inspection.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant DesktopPreReadyPlatform
  participant fs.statSync
  participant Electron.app
  participant safeStorage
  DesktopPreReadyPlatform->>fs.statSync: Inspect the legacy profile
  fs.statSync-->>DesktopPreReadyPlatform: Return whether the profile exists
  DesktopPreReadyPlatform->>Electron.app: Set userData to the selected profile
  DesktopPreReadyPlatform->>Electron.app: Register the ready listener
  Electron.app->>safeStorage: Check encryption availability on ready
Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 6064b

On Windows, startup now chooses the existing legacy or current profile directory before startup can yield to Electron readiness, and checks encryption availability once Electron is ready. A fresh install still starts normally, because Electron accepts a profile directory that does not exist yet. No outstanding issues block merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6064b

The change addresses a sensitive startup-ordering issue for encrypted local data. The reviewed paths do not establish a new remote route to that data, but handling of an error during the new readiness check remains uncertain.

Retained concerns

  • Low · reliability · inferred: If the new ready-time native availability call throws, its callback has no demonstrated error containment. This could interrupt startup access to the encrypted catalog, although a throwing condition after ready has not been established.
Security review details

Security Blast Radius

  • inferred — The evidenced inputs to profile choice are the local process environment and legacy-profile filesystem state. The reviewed relationships do not establish a new renderer- or remote-controlled path to the selection.

Trust Boundaries and Controls

  • inferred — Selecting userData earlier does not itself remove the catalog's independent safeStorage availability and decryption checks. The new ready-time probe is separate from those checks.

Resilience and Maintainability Implications

  • inferred — Normal ordering and inspection failure are tested, but the available evidence does not prove profile stability across every readiness, interruption, or repeated-startup transition.

Hardening Proposals

  • proposed — Define explicit failure handling for the ready-time availability probe, consistent with the existing wrapped safeStorage calls, and verify that later userData setters retain the selected profile.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: selecting the Windows encryption profile before startup yields.
Description check ✅ Passed The description includes the required What Changed, Why, and Checklist sections. It also documents validation, scope, platform behavior, and why UI evidence is not applicable.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
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 unit tests (beta)
  • Create a new PR

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant