Sign up
View stargazers13Sign up

refactor(shared): move the node:sqlite Effect SQL client into shared#7272

merged2
browser-profiles
shared-sqlite-client
openedAug 16, 2026byJJulius Marminge
22 files changed
View on GitHubTwo-way sync enabledPR last imported 3d ago

Stacked on #7254 — review that first; this PR's base is browser-profiles.

The desktop app needs to read browser cookie databases and should reach for the same Effect SQL client the server already uses, rather than opening node:sqlite by hand. Nothing about the client is server-specific, so it moves to packages/shared and both apps import it from there.

Pure move plus import updates — the client itself is unchanged. Split out of #7255 so the cookie-import work reviews on its own.

Model: Claude Opus 5 (1M context), harness: Claude Code.

🤖 Generated with Claude Code

Note

Low Risk Refactor-only with no client logic changes; risk is limited to missed imports still pointing at the removed server path.

Overview Relocates the Effect SQL client built on node:sqlite from server persistence into @t3tools/shared/nodeSqliteClient (lowercase module name) so desktop and other packages can reuse the same layer without duplicating SQLite wiring.

packages/shared gains a ./nodeSqliteClient export; the implementation and its unit test live under packages/shared/src/. apps/server updates every consumer—dev DB and sqlite-state CLIs, migration tests, and the Node runtime loader in Sqlite.ts—to import from the shared package instead of ../NodeSqliteClient. Behavior of the client is unchanged; this is an import-path and packaging move.

Reviewed by Cursor Bugbot for commit ae568f8bd04fb8734dc76529f42363c8f544c1bf. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Move NodeSqliteClient into @t3tools/shared package

  • Moves NodeSqliteClient.ts and its test from apps/server/src/persistence/ to packages/shared/src/ (renamed to lowercase nodeSqliteClient), and adds a ./nodeSqliteClient export path in package.json
  • Updates all imports across apps/server scripts, migration tests, and the defaultSqliteClientLoaders loader to use @t3tools/shared/nodeSqliteClient instead of relative paths
  • Disables browser profile create/rename/remove and default-profile changes in IntegrationsSettings.tsx until client settings are hydrated, preventing premature mutations
  • Risk: all consumers must now use the scoped import @t3tools/shared/nodeSqliteClient; any remaining references to the old apps/server/src/persistence/NodeSqliteClient.ts path will fail to resolve

Macroscope summarized ae568f8.

CcoderabbitaiBotAug 16, 2026
Synced from GitHub

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b93b4a04-c740-44b5-b1fb-f238ae669670

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

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

MmacroscopeappBotreviewedAug 16, 2026
Synced from GitHub

Reviewed the changed Effect service code (desktop BrowserSession/PreviewManager, server ProviderService, the NodeSqliteClient move) against the service conventions. Service definition order, inline Context.Service interfaces, make/layer exports, Foo["Service"] references, environment-based dependency acquisition, and the pure module move with no compatibility shims all look consistent. One import-convention deviation in the new contracts module is noted inline.

Posted via Macroscope — Effect Service Conventions

MmacroscopeappBotreviewedAug 16, 2026
Synced from GitHub

One finding: the new leading-icon utility on MenuSubTrigger also matches the chevron on sub-triggers that have no leading icon, which regresses the two existing icon-less sub-triggers (DiffPanel "Turn", PreviewMoreMenu "Appearance"). Details inline.

Posted via Macroscope — UI Consistency

MmacroscopeappBotAug 16, 2026
Synced from GitHub

Correction to my previous review comment: the inline note referenced packages/contracts/src/browserProfile.ts, which belongs to the base branch (browser-profiles) and is not part of this pull request's changed files. Within this PR's actual scope — moving the node:sqlite Effect SQL client to @t3tools/shared/nodeSqliteClient and updating its importers — there are no Effect service convention findings: the move is pure, no compatibility re-export shim was left behind, every consumer was updated to the canonical path, and all imports use subpath namespace form.

Posted via Macroscope — Effect Service Conventions

MmacroscopeappBotAug 16, 2026
Synced from GitHub

UI consistency finding (inline comment could not be attached — apps/web/src/components/ui/menu.tsx is part of the stacked browser-profiles base, not this PR's own file list, so posting it here instead).

apps/web/src/components/ui/menu.tsx — MenuSubTrigger leading-icon utility also hits the chevron

The new class string starts with [&>svg:first-of-type]:-mx-0.5. That only excludes the chevron when the trigger has a leading icon. On the two existing icon-less sub-triggers — DiffPanel's "Turn" and PreviewMoreMenu's "Appearance" — the chevron is the first svg of its type, so .trigger > svg:first-of-type (specificity 0,2,1) outranks the chevron's own ms-auto / -me-0.5 classes (0,1,0) and applies margin-inline: -0.125rem. ms-auto is overridden, so the chevron stops being pinned to the right edge and collapses next to the label.

Smallest fix: scope the utilities away from the last child, which never matches the chevron and behaves identically for real leading icons:

diff
1- "[&>svg:first-of-type]:-mx-0.5 flex min-h-8 cursor-pointer items-center gap-2 ... [&>svg:first-of-type:not([class*='opacity-'])]:opacity-80 [&_svg]:pointer-events-none [&>svg]:shrink-0",
2+ "[&>svg:not(:last-child)]:-mx-0.5 flex min-h-8 cursor-pointer items-center gap-2 ... [&>svg:not(:last-child):not([class*='opacity-'])]:opacity-80 [&_svg]:pointer-events-none [&>svg]:shrink-0",

(the ... stands for the unchanged middle of the existing class string). Worth a quick check of the emitted CSS plus the "Appearance" and "Turn" submenus after the change.

Posted via Macroscope — UI Consistency

MmacroscopeappBotAug 16, 2026
Synced from GitHub

Approvability

Verdict: Approved at a999041

Macroscope's review found this PR approvable — This PR is a mechanical relocation of an unchanged SQLite client into the shared package, with corresponding export and import updates. The production loader continues to use the same implementation, and no product defaults or user-facing behavior change.

No code changes detected at ae568f8. Prior analysis still applies.

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

Ggithub-actionsBotAug 16, 2026
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire—13.3 KiB—15.1 KiB✅
CodexThread snapshot wire—6.9 KiB—7.3 KiB✅
CodexLive turn WebSocket wire—6.4 KiB—7.8 KiB✅
CodexLive turn WebSocket decoded—55.6 KiB—66.4 KiB✅
CodexLive turn messages—10—21✅
ClaudeTotal thread wire—13.2 KiB—15.1 KiB✅
ClaudeThread snapshot wire—6.9 KiB—7.3 KiB✅
ClaudeLive turn WebSocket wire—6.3 KiB—7.8 KiB✅
ClaudeLive turn WebSocket decoded—56.3 KiB—66.4 KiB✅
ClaudeLive turn messages—8—21✅

Baseline: unavailable · PR result: ae568f8 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Added 2 commits

  • refactor(shared): move the node:sqlite Effect SQL client into shared97077d1
  • fix(server): use shared SQLite client in migration testsae568f8