fix: Update @github/copilot-sdk version and enhance version utility t…#840
Conversation
…o locate package.json - Changed @github/copilot-sdk dependency from "^0.1.16" to "0.1.16" in package.json and package-lock.json. - Improved version utility in version.ts to check multiple candidate paths for package.json, ensuring better reliability in locating the file.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the stability and self-sufficiency of the application, particularly for packaged Electron builds. It ensures consistent dependency resolution and enhances the internal mechanism for identifying application metadata, leading to a more reliable and predictable runtime environment. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThis PR updates configuration and enhances backend initialization. It adds a gitignore rule, pins a dependency to a fixed version, improves package.json location resolution with better error handling, and refactors Node executable path selection in the Electron backend to handle dev and packaged builds differently with source tracking. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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 |
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements. It pins the @github/copilot-sdk dependency to a specific version for better stability. The version utility is enhanced to more reliably locate the package.json file across different environments. A significant and valuable change is the refactoring of the backend server startup, which now uses Electron's bundled Node.js runtime for packaged builds, making the application more self-contained. My feedback includes a suggestion to improve error handling for better debuggability.
| const originalError = error instanceof Error ? error.message : String(error); | ||
| throw new Error( | ||
| `Failed to verify Node.js executable at: ${command} (source: ${nodeResult.source}). Reason: ${originalError}` | ||
| ); |
There was a problem hiding this comment.
For better debuggability, it's recommended to preserve the original error's context (including stack trace) when re-throwing. The cause property on Error is perfect for this (available in Node.js v16.9.0+). This change also provides an opportunity to use the new commandSource variable for consistency instead of nodeResult.source.
| const originalError = error instanceof Error ? error.message : String(error); | |
| throw new Error( | |
| `Failed to verify Node.js executable at: ${command} (source: ${nodeResult.source}). Reason: ${originalError}` | |
| ); | |
| throw new Error( | |
| `Failed to verify Node.js executable at: ${command} (source: ${commandSource}).`, | |
| { cause: error } | |
| ); |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ui/src/electron/server/backend-server.ts`:
- Around line 36-62: The dev-mode startup is bypassing full Node discovery by
calling findNodeExecutable({ skipSearch: true, ... }), which returns the literal
'node' and prevents buildEnhancedPath/validation from running; remove the
skipSearch: true option so findNodeExecutable runs platform discovery (keep the
logger usage), then preserve the existing validation logic that checks command
!== 'node' and uses systemPathExists to verify the resolved node path in the
isDev branch (references: findNodeExecutable, buildEnhancedPath, isDev, command,
commandSource).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5473973-a864-4060-a2a3-7262cef54384
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
.gitignoreapps/server/package.jsonapps/server/src/lib/version.tsapps/ui/src/electron/server/backend-server.ts
| if (isDev) { | ||
| // In development, run the TypeScript server via the user's Node.js. | ||
| const nodeResult = findNodeExecutable({ | ||
| skipSearch: true, | ||
| logger: (msg: string) => logger.info(msg), | ||
| }); | ||
| command = nodeResult.nodePath; | ||
| commandSource = nodeResult.source; | ||
|
|
||
| // Validate that the found Node executable actually exists | ||
| // systemPathExists is used because node-finder returns system paths | ||
| if (command !== 'node') { | ||
| let exists: boolean; | ||
| try { | ||
| exists = systemPathExists(command); | ||
| } catch (error) { | ||
| const originalError = error instanceof Error ? error.message : String(error); | ||
| throw new Error( | ||
| `Failed to verify Node.js executable at: ${command} (source: ${nodeResult.source}). Reason: ${originalError}` | ||
| ); | ||
| } | ||
| if (!exists) { | ||
| throw new Error( | ||
| `Node.js executable not found at: ${command} (source: ${nodeResult.source})` | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the call site disables discovery and that the helper returns the fallback literal.
rg -n -C2 'skipSearch:\s*true' apps/ui/src/electron/server/backend-server.ts
sed -n '275,300p' libs/platform/src/node-finder.tsRepository: AutoMaker-Org/automaker
Length of output: 952
🏁 Script executed:
# Check other call sites of findNodeExecutable to understand the pattern
rg -n 'findNodeExecutable' apps/ui/src/electron/server/backend-server.ts libs/platform/src/node-finder.ts --type tsRepository: AutoMaker-Org/automaker
Length of output: 751
🏁 Script executed:
# Check if there are comments or documentation explaining the skipSearch choice
rg -B5 -A5 'skipSearch.*true' apps/ui/src/electron/server/ --type tsRepository: AutoMaker-Org/automaker
Length of output: 887
🏁 Script executed:
# Verify the function signature and default behavior
rg -B3 -A10 'export function findNodeExecutable' libs/platform/src/node-finder.ts --type tsRepository: AutoMaker-Org/automaker
Length of output: 477
🏁 Script executed:
# Check the packaged mode branch to see the alternative approach
rg -B5 -A15 'if (isDev)' apps/ui/src/electron/server/backend-server.ts --type tsRepository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Look for any environment setup or PATH configuration in the Electron context
rg -n 'NODE_PATH|process.env.PATH|ELECTRON' apps/ui/src/electron/ --type ts | head -20Repository: AutoMaker-Org/automaker
Length of output: 574
🏁 Script executed:
# Get the full context of the backend server startup including buildEnhancedPath usage
sed -n '1,170p' apps/ui/src/electron/server/backend-server.ts | tail -40Repository: AutoMaker-Org/automaker
Length of output: 1614
🏁 Script executed:
# Look for buildEnhancedPath implementation and documentation
rg -B3 -A15 'export.*buildEnhancedPath|function buildEnhancedPath' libs/platform/src/ --type tsRepository: AutoMaker-Org/automaker
Length of output: 1418
Don't bypass findNodeExecutable() in dev mode.
With skipSearch: true, the function returns the fallback literal 'node' and skips all platform discovery. Additionally, buildEnhancedPath() explicitly skips PATH enhancement when receiving the fallback value, and validation is skipped at lines 47–62. In dev mode, Electron will rely solely on the current process.env.PATH to resolve node, which is often unavailable outside shell-launched sessions. Use the same discovery logic as packaged mode by removing skipSearch: true.
♻️ Proposed fix
const nodeResult = findNodeExecutable({
- skipSearch: true,
logger: (msg: string) => logger.info(msg),
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/src/electron/server/backend-server.ts` around lines 36 - 62, The
dev-mode startup is bypassing full Node discovery by calling
findNodeExecutable({ skipSearch: true, ... }), which returns the literal 'node'
and prevents buildEnhancedPath/validation from running; remove the skipSearch:
true option so findNodeExecutable runs platform discovery (keep the logger
usage), then preserve the existing validation logic that checks command !==
'node' and uses systemPathExists to verify the resolved node path in the isDev
branch (references: findNodeExecutable, buildEnhancedPath, isDev, command,
commandSource).
…o locate package.json
Summary by CodeRabbit
Release Notes