Sign up
View stargazersSign up

fix(desktop): recover unparseable connection catalogs#25549

open1
main
maria/takeover-pr-24844
opened3d agobyMmaria
2 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Problem

Malformed connection-catalog.json bytes reject desktop catalog hydration and prevent access to otherwise healthy environments and threads.

Change

Preserve the exact failed bytes in a SHA-256-named backup before recovering readable legacy credentials or returning an empty catalog. Recheck the active file after backup so a concurrent save wins. Backup failure still rejects; unsupported envelopes, filesystem errors, invalid base64, and decryption failures keep their existing behavior.

Reuse @Zheruel's recovery implementation and focused tests. Flatten the read helper, compile the write-error schema predicate once, and include truncated JSON in the malformed-byte fixtures. No renderer, IPC contract, or normal save behavior changes. Remote connections absent from readable legacy records need re-adding; the backup path is logged.

Scope and approval

Maintainer-requested takeover of #24844 for the malformed-byte recovery portion of pingdotgg/t3code#4750. This changes one desktop persistence module and its existing tests. The current writer already uses temporary-file replacement; the original corruption trigger remains unproven. Final human review and approval are still required.

Verification

  • Regression tests against current main: 5 failed, 5 passed; malformed catalog reads reject instead of recovering.
  • All 29 PR checks passed; CodeRabbit found no actionable issues, and both independent code reviewers found no issues at bd7ffbacefe555617d6b45218adb9b1a3cef6422. Macroscope requires human approval.
  • Blacksmith vp run check:changed: format, lint, both knip passes, affected typechecks, and all 10 catalog-store tests passed.
  • Electron 44.9.3 through real renderer IPC: main rejects NUL bytes without writing; replacement recovers, six concurrent reads preserve one byte-identical backup, and native safeStorage save/reread succeeds.
  • Before/after runtime transcript.
  • Unverified: OS keychains (runtime used isolated Linux basic_text), Windows/WSL, macOS, packaged client hydration/reconnection, and physical power loss. No rendering code changed, so evidence is a native IPC transcript.

Takeover

Replaces #24844, with thanks to @Zheruel for the original implementation.

Built with GPT-6.1-Sol in the Codex harness on T3 Code.

Checks

23 successful · 9 skipped · 1 neutral
All checks passed
bd7ffba

Added 1 commit

  • fix(desktop): recover unparseable connection catalogsbd7ffba
MmacroscopeappBot3d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — The production store now performs durable corruption backups, concurrent-save handling, and legacy credential recovery for malformed connection catalogs. This is a contained change with focused tests, but its nontrivial persistence behavior and sensitive connection-data handling warrant human review.

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

CcoderabbitaiBot3d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 3f9448747bb8848050dc2cacf83affa6f94a66a1 and bd7ffbacefe555617d6b45218adb9b1a3cef6422.

External image Files selected for processing: 2
  • apps/desktop/src/app/DesktopConnectionCatalogStore.test.ts
  • apps/desktop/src/app/DesktopConnectionCatalogStore.ts

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

Walkthrough

The catalog store now reads and writes catalog bytes directly. When JSON decoding fails, it preserves the original bytes in a digest-named backup and rereads the catalog before fallback. It also loads legacy data when the catalog is absent or unchanged after recovery.

Changes: Desktop catalog recovery

Layer / File(s)Summary
Catalog byte I/O and encoding
apps/desktop/src/app/DesktopConnectionCatalogStore.ts		The store reads raw bytes, encodes catalog documents before writing, and can sync a temporary file. Read and write failures receive operation-specific errors.
Malformed catalog recovery and migration
apps/desktop/src/app/DesktopConnectionCatalogStore.ts, apps/desktop/src/app/DesktopConnectionCatalogStore.test.ts		The store backs up malformed bytes, rereads the catalog to detect replacement, and loads legacy data when applicable. Tests cover malformed input, backup failures, filesystem errors, and concurrent replacement.

Priority: External image Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Possibly related PRs

  • pingdotgg/t3code#24844: Implements related catalog recovery behavior; this PR replaces and extends it with raw-byte reads, backups, concurrent-save checks, and write-error handling.

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · up to bd7ff

No new merge-blocking issue was established. The reported platform and packaged-client testing gaps remain unverified rather than evidence of a failure.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title "fix(desktop): recover unparseable connection catalogs" follows conventional commit format and matches the main change. The PR preserves malformed connection-catalog.json bytes in a SHA-256-…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Description checkPassedThe pull request includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the failure, the recovery behavior, scope limits, approval context, focused test res…
Full details: Title check

Explanation

The title "fix(desktop): recover unparseable connection catalogs" follows conventional commit format and matches the main change. The PR preserves malformed connection-catalog.json bytes in a SHA-256-named backup, then recovers readable legacy credentials or returns an empty catalog. The title is specific and short, and it names the desktop scope.

Full details: Description check

Explanation

The pull request includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the failure, the recovery behavior, scope limits, approval context, focused test results, runtime verification, and unverified environments.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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