Synced from GitHub
fix(desktop): recover from malformed connection catalogs#13468
What Changed
A malformed connection-catalog.json currently fails the desktop catalog IPC call and prevents the client from registering its local connection. This change preserves the damaged file under a unique .corrupt.<pid>.<uuid> name, then uses the existing missing-catalog path to restore legacy records or start an empty catalog.
Recovery succeeds only after the damaged file is preserved. Read, base64, and decryption failures keep their existing error behavior. Catalog reads, writes, and clears share a semaphore to prevent recovery from moving a concurrent replacement. Writes flush and close the temporary file before replacing the previous catalog, with a writable handle for Windows.
Why
Fixes #4750.
Supporting investigation: Windows 10 incident evidence and isolated reproduction, including catalog contents, startup errors, database checks, and the distinction between confirmed recovery failure and the unproven corruption trigger.
The reported restart left a 1,552-byte, zero-filled catalog while the project and thread database remained readable. That document alone blocked startup. Preserving it and restoring the normal missing-catalog behavior lets the desktop reconnect without changing project or thread data. Remote connection details cannot be reconstructed from zero-filled bytes unless legacy records remain available.
Verification
- Five regression cases failed against the original implementation, then passed with recovery: zero-filled, empty, invalid JSON, truncated JSON, and invalid envelope fields.
- 102 focused repository tests passed across desktop catalog/legacy migration/IPC, web catalog storage, first-run logic, and connection registry suites. Two additional isolated desktop-to-renderer checks passed, including failed quarantine without a renderer overwrite.
- Failure coverage includes write, flush, and rename failures, retry, concurrent reads and writes, legacy relay/SSH/bearer migration, and preserved read/decryption errors.
- Desktop type checking, targeted lint/formatting, whitespace checks, and the full desktop build passed on Windows with Node 24.13.1.
- Computer Use verified the built desktop against an isolated database snapshot and a copy of the damaged catalog. Existing threads and Settings loaded. The corrupt bytes remained unchanged in quarantine. Native Windows encryption passed inside the real desktop process; after a full app restart, the saved catalog remained unchanged and thread search worked.
No visual UI changes. Computer Use used accessibility and keyboard controls because screenshot capture was unavailable on this Windows 10 host. Physical power-loss behavior was not tested; flushing does not establish a power-loss guarantee.
Checklist
- This PR is focused on connection catalog recovery and write durability.
- I explained what changed and why.
Written with assistance from GPT-6 in Codex.
Summary by CodeRabbit
- Bug Fixes
- Malformed connection catalogs are now quarantined during recovery, preserving their original contents instead of treating them as empty.
- Catalog reads, writes, and clears are coordinated to prevent conflicting operations during recovery.
- Catalog updates are synchronized before replacing the existing catalog. If an update fails, the previous catalog remains available and the operation can be retried.
Added 1 commit
- fix(desktop): recover from malformed connection catalogs05709f3
Synced from GitHub
Approvability
Verdict: Approved at 05709f3
Macroscope's review found this PR approvable — This is a narrowly scoped desktop persistence bug fix that safely quarantines malformed catalog envelopes, preserves the original bytes, serializes recovery with catalog operations, and flushes atomic writes. Existing valid-catalog and decryption-error behavior remains unchanged, with extensive targeted regression coverage.
No code changes detected at f7f8aa8. Prior analysis still applies.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
Navigate logical layers of code changes, visualize relationships, and explore their blast radius.
Important
Review skipped
Review was skipped as selected files did not have any reviewable changes.
⚙️ Run configuration
Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: 49f729d4-1402-4bbb-a2f8-c47378f5cc8b
📥 Commits
Reviewing files that changed from the base of the PR and between 05709f34f990e131e561d95ad4a8d029cdbfadcb and f7f8aa801d8170e0ceabab57dd67b65ee28506a4.
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
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: fcbc26ae-0897-4e01-9ed5-32e1ab1e8d47
📥 Commits
Reviewing files that changed from the base of the PR and between f26ee083fe41720207fb10616359500dcecc19d3 and 05709f34f990e131e561d95ad4a8d029cdbfadcb.
📒 Files selected for processing (2)
apps/desktop/src/app/DesktopConnectionCatalogStore.test.tsapps/desktop/src/app/DesktopConnectionCatalogStore.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📝 Walkthrough
Walkthrough
The connection catalog store now serializes reads, writes, and clears. It quarantines malformed catalog files and syncs temporary files before replacing the catalog. Tests cover recovery, write failures, and retries.
Changes
Connection catalog store
| Layer / File(s) | Summary |
|---|---|
Serialized catalog recovery apps/desktop/src/app/DesktopConnectionCatalogStore.ts, apps/desktop/src/app/DesktopConnectionCatalogStore.test.ts | The store serializes catalog operations and quarantines malformed documents. Recovery errors identify quarantine failures. Tests cover migration, IPC reads and writes, concurrent operations, and preservation of corrupt file contents. |
Synced catalog writes apps/desktop/src/app/DesktopConnectionCatalogStore.ts, apps/desktop/src/app/DesktopConnectionCatalogStore.test.ts | The store syncs the temporary file before replacing the catalog and reports sync failures as write errors. Tests verify failure handling, preservation of the prior catalog, and successful retry. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Suggested reviewers: juliusmarminge
Merge Risk: ⚪ Minimal · up to 05709
The catalog recovery and write changes are ready to merge after normal checks.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check | ❓ Inconclusive | The PR implements the main [#4750] recovery work. It writes through a temporary file, syncs and closes the temporary handle, then renames it. It serializes reads, writes, and clears. It quarantines ma… | Provide reviewable evidence for the unchanged-content write check and the UI path that presents catalog read failures. If either behavior already exists before this pull request, identify the relevant implementation and tests. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Out of Scope Changes check | ✅ Passed | The changed production code and tests stay within [#4750]. They address atomic catalog writes, malformed-file quarantine, recovery serialization, legacy fallback, and failure handling. No unrelated pr… |
| 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… |
| Title check | ✅ Passed | The title clearly and concisely describes the main change: recovery from malformed desktop connection catalogs. |
| Description check | ✅ Passed | The description explains what changed, why it changed, verification results, and the checklist status. The UI Changes section is omitted because the author states that the PR has no visual UI changes. |
Full details: Linked Issues check
Explanation
The PR implements the main [#4750] recovery work. It writes through a temporary file, syncs and closes the temporary handle, then renames it. It serializes reads, writes, and clears. It quarantines malformed catalog documents under a unique name, preserves their bytes, and follows the missing-catalog path for legacy migration or an empty catalog. The tests cover malformed inputs, migration, retry after quarantine failure, concurrent access, and write-stage failures. The available evidence does not establish two remaining [#4750] requirements at the reviewed head: that unchanged serialized content skips a write, and that catalog read failures are surfaced in the UI. The summary only states that existing read-error behavior is retained.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create a new PR
Comment @coderabbitai help to get the list of available commands.
Added 1 commit
- Merge branch 'main' into fix/desktop-catalog-recoveryf7f8aa8
Synced from GitHub
Additional validation on the affected Windows 10 installation:
- Applied the catalog implementation from
05709f34fas a local backport to the installed 0.0.42 /719a76ca1dbf/ Electron 44.1.0 release. Only the compiled catalog-service region changed. The other 147 archived files, native unpacked files, executable, server, and UI were verified unchanged. The original archive, damaged catalog, and a consistent database snapshot were backed up first. - Tested a copy of that installed application before replacement. With the real database snapshot and corrupt-file copy, it recovered, loaded the existing threads, and exposed them through Computer Use. The database snapshot passed
PRAGMA quick_check. - Closed the real desktop normally, replaced its application archive, and reopened the installed executable. The application itself quarantined the live 1,552-byte NUL-filled catalog through
desktop:get-connection-catalog; the installer did not reset it manually. - The live quarantine matches the original byte for byte (SHA-256
07cb830e7cb76dc7d43b0fc8bc389e8225057335ffe80dda35b8c38e4b24f2c1). The installed archive matches the tested replacement. The local environment endpoint returns HTTP 200, version 0.0.42, platform Windows x64.
Two remaining observations distinguish the recovery result from other failures:
- The installation helper's final check initially failed because it attempted to parse an empty startup log. The archive had already been replaced and the app restarted successfully. The empty-log handling was corrected locally, and the installation hashes, quarantine, and backend health were then verified directly.
- The existing
docker-desktopWSL environment still reportsNo space left on devicewhile staging its runtime and a missing Node.js/node-pty prerequisite. T3 logs its fallback after repeated failures; the native Windows backend is healthy. That WSL problem remains unresolved by this catalog fix.
The final live UI could not be inspected with Computer Use: Windows returned GetCursorPos failed: Access is denied (0x80070005) after refreshing the window and retrying. The copied-installation UI check passed, but the live result above is based on the installed archive, the actual renderer catalog call, preserved file bytes, and backend health. No physical power-loss test or live remote-host connection test was performed.
Synced from GitHub
Another reproduction of #4750 on current main (148e6deea0, Windows 11 build 26300) after a power cut. Details are in https://github.com/pingdotgg/t3code/issues/4750. I checked the same quarantine approach locally against main and it fixes the case. Would be good to see this land.
Note
Written by claude-opus-5-5 on behalf of Maria
Hi! We are cleaning up open PRs, and this one appears to have been created with an older model (gpt-6). If this change is really important, we recommend rebuilding the PR with a newer model if possible.