fix(desktop): unbreak safe-url test on bun by splitting pure helpers

[Kitenite]

Summary

• apps/desktop/src/main/lib/safe-url/safe-url.ts imports shell from electron at the module top level, so bun's test runner (CI) fails loading the module with SyntaxError: Export named 'shell' not found in module '.../electron/index.js'. This broke the desktop test job on main after fix(desktop): allowlist URL schemes before shell.openExternal #3650 merged (see run 24802754721).
• Moved the pure URL helpers (isSafeExternalUrl, externalUrlLogLabel) into scheme.ts. They have no electron dependency and are what the test actually exercises.
• safeOpenExternal stays in safe-url.ts with the electron import; it imports the pure helpers from scheme.ts. The safe-url/index.ts barrel keeps the same three exports, so no callers need to change.
• Updated the test to import from ./scheme so loading it doesn't pull electron.

Test plan

• bun test src/main/lib/safe-url/ — 5 pass locally
• bun run typecheck in apps/desktop
• bun run lint apps/desktop
• CI green on this PR (it should also unstick any open branch that has been failing on this test since fix(desktop): allowlist URL schemes before shell.openExternal #3650 landed)

---

Summary by cubic

Fixes Bun test failures in desktop by splitting pure URL helpers from Electron code in safe-url. Tests now import the pure module to avoid loading electron, while the public API stays the same.

• Bug Fixes
  • Moved isSafeExternalUrl and externalUrlLogLabel to scheme.ts (no electron).
  • Kept safeOpenExternal in safe-url.ts and reused the helpers.
  • Updated the barrel to export helpers from scheme; tests import ./scheme.

^{Written for commit 4330533. Summary will update on new commits.}

Summary by CodeRabbit

• Refactor
  • Reorganized internal URL validation logic for improved code structure and maintainability.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cd968b7c-882b-4c51-9ca9-5920fc2248db

📥 Commits

Reviewing files that changed from the base of the PR and between e49600f and 4330533.

📒 Files selected for processing (4)

• apps/desktop/src/main/lib/safe-url/index.ts
• apps/desktop/src/main/lib/safe-url/safe-url.test.ts
• apps/desktop/src/main/lib/safe-url/safe-url.ts
• apps/desktop/src/main/lib/safe-url/scheme.ts

---

📝 Walkthrough

Walkthrough

The safe-url module is reorganized by extracting scheme validation and logging functions into a dedicated file. Two functions (isSafeExternalUrl and externalUrlLogLabel) move from safe-url.ts to a new scheme.ts file. The core safeOpenExternal function now imports these utilities from their new location. Re-exports in the index file ensure the public API remains unchanged.

Changes

Cohort / File(s)	Summary
Scheme Utilities apps/desktop/src/main/lib/safe-url/scheme.ts	New file introducing isSafeExternalUrl and externalUrlLogLabel functions, defining protocol whitelist (http, https, mailto) and validation/logging logic.
Core Refactoring apps/desktop/src/main/lib/safe-url/safe-url.ts	Removes isSafeExternalUrl and externalUrlLogLabel definitions; imports them from ./scheme; safeOpenExternal functionality unchanged.
Export Re-organization apps/desktop/src/main/lib/safe-url/index.ts	Updates export sources: externalUrlLogLabel and isSafeExternalUrl now sourced from ./scheme instead of ./safe-url.
Test Dependency Update apps/desktop/src/main/lib/safe-url/safe-url.test.ts	Updates import source for externalUrlLogLabel and isSafeExternalUrl from ./scheme instead of ./safe-url.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A scheme took its place, a home of its own,
The safe-url grew cleaner, each function well-known,
We hopped through the modules with purposeful care,
Now protocols nestle in their rightful lair! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing a Bun test failure by refactoring safe-url helpers into a separate module.
Description check	✅ Passed	The description comprehensively covers the problem, solution, and testing performed, though it deviates from the template structure with auto-generated content.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-safe-url-electron-import

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR fixes a CI breakage in the desktop test job caused by safe-url.ts importing shell from electron at the module top level. Bun's test runner cannot load the Electron module in CI, so any test that transitively imported safe-url.ts would fail with a SyntaxError.

The fix cleanly separates concerns:

• Pure URL helpers (isSafeExternalUrl, externalUrlLogLabel) are moved into a new scheme.ts file with zero Electron dependency.
• safe-url.ts retains only safeOpenExternal (which actually needs shell) and now imports the helpers from scheme.ts.
• The index.ts barrel is updated to re-export from both modules, keeping the public API identical — no call-sites change.
• The test file is updated to import directly from ./scheme, avoiding the Electron module entirely.

The change is minimal, surgical, and does not alter any runtime behaviour.

Confidence Score: 5/5

Safe to merge — pure refactor with no behaviour change and a clean public-API surface.

The helper logic is moved verbatim (no edits), the barrel preserves all three exports, and the only test that exercises these functions now correctly imports from the electron-free module. There are no logic changes, no new dependencies, and no risk to callers.

No files require special attention.

Important Files Changed

Filename	Overview
apps/desktop/src/main/lib/safe-url/scheme.ts	New file containing pure URL helpers moved verbatim from safe-url.ts; no Electron dependency, fully testable in bun CI.
apps/desktop/src/main/lib/safe-url/safe-url.ts	Stripped of pure helpers; now only holds safeOpenExternal (which legitimately needs electron) and imports helpers from scheme.ts.
apps/desktop/src/main/lib/safe-url/index.ts	Barrel updated to re-export from both safe-url.ts and scheme.ts; public API unchanged.
apps/desktop/src/main/lib/safe-url/safe-url.test.ts	Import path updated from ./safe-url to ./scheme so bun can load the test without pulling in the Electron module.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["index.ts (barrel)"] -->|"re-exports safeOpenExternal"| B["safe-url.ts\n(needs electron)"]
    A -->|"re-exports isSafeExternalUrl\nexternalUrlLogLabel"| C["scheme.ts\n(pure helpers, no electron)"]
    B -->|"imports helpers from"| C
    D["safe-url.test.ts\n(bun CI)"] -->|"imports directly from"| C
    B --> E["electron shell.openExternal"]

Loading

_{Reviews (1): Last reviewed commit: "fix(desktop): unbreak safe-url test on b..." | Re-trigger Greptile}

[cubic-dev-ai]

No issues found across 4 files

[github-actions]

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

• ⚠️ Neon database branch

Thank you for your contribution! 🎉