fix(desktop): handle characters around paths in terminal#724
Conversation
Capture stderr from spawn process to surface actual error messages from the `open` command instead of generic "exited with code 1". Before: "'open' exited with code 1. The application may not be installed." After: "Unable to find application named 'Cursor'" (actual error from macOS) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughA new helper function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
…file paths
Add path cleaning to handle common terminal output patterns when opening files:
- Strip wrapper characters: quotes ("'`), parentheses, brackets
- Strip trailing punctuation: . , : ; ? ! (but preserve extensions and line:col)
- Handle nested wrappers and wrappers with trailing punctuation
Examples: "(./file.ts)." → "./file.ts", '"./file.ts",' → "./file.ts"
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![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
🤖 Fix all issues with AI agents
In @apps/desktop/src/lib/trpc/routers/external/index.ts:
- Line 11: Import/usage mismatch: the code imports resolvePathWithFallback but
the router calls resolvePath (and the helpers module only exports resolvePath),
so fix by making the import and usage match; either change the named import
resolvePathWithFallback to resolvePath in the import list or change the call
site to resolvePathWithFallback if you add that export to the helpers module.
Locate the import statement referencing resolvePathWithFallback and replace it
with resolvePath, or alternatively export resolvePathWithFallback from the
helpers module to match the call to resolvePathWithFallback, ensuring the
function name is consistent with the call site (resolvePath).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/lib/trpc/routers/external/helpers.test.tsapps/desktop/src/lib/trpc/routers/external/helpers.tsapps/desktop/src/lib/trpc/routers/external/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/lib/trpc/routers/external/helpers.tsapps/desktop/src/lib/trpc/routers/external/helpers.test.tsapps/desktop/src/lib/trpc/routers/external/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/lib/trpc/routers/external/helpers.tsapps/desktop/src/lib/trpc/routers/external/helpers.test.tsapps/desktop/src/lib/trpc/routers/external/index.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/lib/trpc/routers/external/helpers.tsapps/desktop/src/lib/trpc/routers/external/helpers.test.tsapps/desktop/src/lib/trpc/routers/external/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/lib/trpc/routers/external/helpers.tsapps/desktop/src/lib/trpc/routers/external/helpers.test.tsapps/desktop/src/lib/trpc/routers/external/index.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate tests with implementation files using .test.ts or .test.tsx suffix
Files:
apps/desktop/src/lib/trpc/routers/external/helpers.test.ts
🧠 Learnings (7)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules like node:path or dotenv in electron-router-dom.ts and similar shared files - they run in both main and renderer processes
Applied to files:
apps/desktop/src/lib/trpc/routers/external/helpers.tsapps/desktop/src/lib/trpc/routers/external/helpers.test.tsapps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Extract business logic from tRPC procedures into utility functions when logic exceeds ~50 lines, is used by multiple procedures, or needs independent testing
Applied to files:
apps/desktop/src/lib/trpc/routers/external/helpers.tsapps/desktop/src/lib/trpc/routers/external/helpers.test.ts
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/*.ts : Never import Node.js modules in shared code like electron-router-dom.ts - it runs in both main and renderer processes
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to **/*.{ts,tsx} : Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
🧬 Code graph analysis (1)
apps/desktop/src/lib/trpc/routers/external/helpers.test.ts (1)
apps/desktop/src/lib/trpc/routers/external/helpers.ts (2)
resolvePath(149-177)stripPathWrappers(115-142)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (8)
apps/desktop/src/lib/trpc/routers/external/helpers.ts (5)
43-54: LGTM!The wrapper pairs constant is well-structured and covers common path wrapper characters. Good use of typed tuples for the open/close pairs.
68-102: LGTM! Well-designed helper with good edge case handling.The logic correctly preserves file extensions and line number suffixes while stripping sentence-ending punctuation.
115-142: LGTM!The iterative stripping algorithm correctly handles nested wrappers and trailing punctuation. The loop termination is well-guarded with both the
changedflag and length check.
149-151: LGTM!Good integration point - stripping wrappers first ensures subsequent path operations work on clean input.
183-214: LGTM! This change directly addresses the PR objective.Capturing stderr and surfacing the actual OS error message (e.g., "Unable to find application named 'Cursor'") is a significant UX improvement over the generic "exited with code 1" message. The optional chaining on
child.stderr?.oncorrectly handles the edge case where stderr might be null.apps/desktop/src/lib/trpc/routers/external/helpers.test.ts (3)
4-4: LGTM!Good practice to import and test the exported
stripPathWrappersdirectly alongsideresolvePath.
215-265: LGTM! Thorough integration tests.Good coverage of wrapper stripping combined with other
resolvePathfeatures like~expansion and relative path resolution.
268-428: LGTM! Excellent test coverage.The test suite thoroughly covers:
- All wrapper types individually
- Nested and mixed wrappers
- Critical edge cases (mismatched wrappers, internal wrappers, empty input)
- Trailing punctuation with proper preservation of extensions and line numbers
- Combined scenarios (wrappers + trailing punctuation)
Kitenite
changed the title
Add tests for: - Line numbers with trailing punctuation (file.ts:42. → file.ts:42) - Line:col with trailing punctuation - Various extension types (.mp3, .c, .TSX) - Dotfiles (.gitignore, .eslintrc.json) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
opencommand",',`), parentheses, brackets. , : ; ? !) while preserving extensions and line:col suffixes"./file.ts".→./file.ts,(./path/file.ts),→./path/file.tsTest plan
"./src/file.ts"- should open correctly(./src/file.ts)- should open correctly./src/file.ts.- should open correctly./src/file.ts:42still work (line number preserved)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.