Fix brain CRDT sync packaging + surface remote runtime integrations#542
Merged
Conversation
Bundles several related fixes on this lane: - Ship cr-sqlite into the static/headless brain. The native tarball (package-native-deps.mjs) now copies crsqlite.<ext> from the desktop vendor dir, and crsqliteExtension.ts resolves it from <ADE_HOME>/runtime/<target>/. Without this the installed brain (the default sync host) had no CRR engine and crashed on crsql_internal_sync_bit for every CRR-table write. - Exclude config-snapshot tables (process_definitions, stack_buttons, test_suites) from CRR — their delete+reinsert rebuild fired CRR triggers; they must never be replicated. - Classify crsql / "not callable" RPC errors as app-level, not transport-level, so a single markCallFailure helper keeps the remote connection green instead of tearing it down. Collapses 9 duplicated catch blocks. - Surface a remote machine's Linear/GitHub/AI when bound to a remote runtime: LinearQuickViewButton no longer suppresses on remote (routes to the remote daemon), LinearSection steers remote to API-key entry (OAuth loopback can't cross remote), plus a RemoteContextBadge. - Don't wipe open project tabs on a transient null project state during remote rebind. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThe PR improves remote runtime stability across native packaging, connection error handling, database schema management, and UI state restoration. It vendors the crsqlite extension, distinguishes transport-level from application-level remote call failures, prevents local tabs from being cleared during remote binding transitions, and adds UI indicators for remote-bound settings and credentials. ChangesRemote runtime and session stability
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
arul28
changed the title
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
June 8, 2026 18:51
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/scripts/package-native-deps.mjs`:
- Around line 227-233: The current check that uses exists(source) returns false
and only logs a warning when the cr-sqlite extension is missing; change this to
fail fast by throwing an Error (or calling process.exit(1)) unless an explicit
env override is set (e.g. ALLOW_MISSING_CRSQLITE=true) so packaging never
produces a known-broken runtime; specifically replace the if (!(await
exists(source))) { ... return false; } block to log the missing source and then
throw new Error(...) when ALLOW_MISSING_CRSQLITE is not truthy, and update the
caller that currently ignores the return (the code at the later check around
line 304) to stop ignoring the outcome and allow the thrown error to propagate
(or check the return and abort) so packaging aborts on missing crsqlite by
default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4a0b752d-474f-47e7-9eec-9cce9cf04b1e
📒 Files selected for processing (14)
apps/ade-cli/scripts/package-native-deps.mjsapps/desktop/src/main/main.tsapps/desktop/src/main/services/remoteRuntime/remoteConnectionService.test.tsapps/desktop/src/main/services/remoteRuntime/remoteConnectionService.tsapps/desktop/src/main/services/state/crsqliteExtension.tsapps/desktop/src/main/services/state/kvDb.test.tsapps/desktop/src/main/services/state/kvDb.tsapps/desktop/src/renderer/components/app/LinearQuickViewButton.tsxapps/desktop/src/renderer/components/app/SettingsPage.tsxapps/desktop/src/renderer/components/app/TopBar.test.tsxapps/desktop/src/renderer/components/app/TopBar.tsxapps/desktop/src/renderer/components/settings/LinearSection.tsxapps/desktop/src/renderer/components/settings/MobilePushPanel.tsxapps/desktop/src/renderer/components/settings/RemoteContextBadge.tsx
- Hard-fail packaging when a required brain target (darwin-arm64) is missing its cr-sqlite extension, instead of silently shipping a runtime that crashes on every CRR write. Non-vendored targets still warn-and-skip. - Replace the misleading silent ".so" fallback in the extension-filename map with an explicit throw for unsupported platforms. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bundles several related fixes landed on this lane:
package-native-deps.mjs) now copiescrsqlite.<ext>from the desktop vendor dir, andcrsqliteExtension.tsresolves it from<ADE_HOME>/runtime/<target>/. Without this the installed brain — the default sync host — had no CRR engine and crashed oncrsql_internal_sync_bitfor every CRR-table write.process_definitions,stack_buttons,test_suites) from CRR. Their delete+reinsert rebuild fired CRR triggers; they must never replicate.crsql/ "not callable" RPC errors as app-level, not transport-level, so a singlemarkCallFailurehelper keeps the remote connection green instead of tearing it down. Collapses 9 duplicated catch blocks.LinearQuickViewButtonno longer suppresses on remote (routes to the remote daemon),LinearSectionsteers remote to API-key entry (OAuth loopback can't cross remote), plus a newRemoteContextBadge.Test plan
crsqliteExtension/kvDbCRR-exclusion unit tests (kvDb.test.ts) — 15/15remoteConnectionServiceerror-classification tests — 14/14TopBarLinear remote-visibility tests — 34/34🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Greptile Summary
This PR bundles five related fixes: ships the cr-sqlite native extension into the static brain package so CRDT replication works on headless installs, excludes config-snapshot tables (
process_definitions,stack_buttons,test_suites) from CRR to stop delete+reinsert rebuilds from firing missing-extension crashes, collapses nine duplicated catch blocks into a singlemarkCallFailurehelper that distinguishes app-level RPC errors from genuine transport failures, and surfaces the remote machine's Linear/GitHub/AI integrations in the UI while blocking OAuth loopback flows that can't cross a remote boundary.package-native-deps.mjscopiescrsqlite.<ext>from the desktop vendor dir;crsqliteExtension.tsadds<ADE_HOME>/runtime/<target>/as a candidate path, with matching directory structure between packager output and resolver input.markCallFailureskipsmergeStatus(error)for app-level errors (crsql, "not callable"), preventing false "host unreachable" toasts and reconnect loops; nine prior duplicated catch blocks now delegate to this helper.main.tsdrops the redundantemitProjectChangedToWindow(null)precursor during remote bind;TopBar.tsxremoves the tab-wiping branch from the!rootPatheffect (delegating tocloseProject()in appStore) and changes session restore to merge rather than replace tabs.Confidence Score: 5/5
Safe to merge — all five fix areas are well-scoped, explicitly commented, and covered by targeted regression tests.
The changes are surgical and each is backed by a concrete test that reproduces the original failure. The markCallFailure refactor correctly preserves the existing isImplicitConnectionFailure gate before touching connection state. The tab-management changes remove a side-effect that was firing too broadly and rely on closeProject() in appStore for authoritative tab clearing — an assumption the new TopBar tests validate directly. The CRR exclusion list additions and the crsqlite packaging path are structurally straightforward. No regressions were found across the changed paths.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[RPC call over established connection throws] --> B{isImplicitConnectionFailure?} B -->|No - app-level error| C[markCallFailure returns early] C --> D[Connection stays connected] B -->|Yes - transport failure| E[mergeStatus: state=error] E --> F[Connection flips to error] D --> G[Error rethrown to caller] F --> GComments Outside Diff (1)
apps/desktop/src/renderer/components/settings/LinearSection.tsx, line 765 (link)useRemoteRuntimeContextfor settings scope — minor consistency nitLinearSectionreadss.projectBinding?.kind === "remote"directly from the store while this PR adds the shareduseRemoteRuntimeContexthook inRemoteContextBadge.tsxfor exactly this purpose. The two are functionally equivalent, but using the hook here would keep the remote-context read consistent across all settings sections and make the hook easier to evolve (e.g. if the condition gains more nuance).Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (2): Last reviewed commit: "ship: harden crsqlite packaging per revi..." | Re-trigger Greptile