Bundle GitHub Copilot CLI for zero-config installation#14
Bundle GitHub Copilot CLI for zero-config installation#14
Conversation
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
…fix comment Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to enable a zero-config Planeteer install by locating and using the GitHub Copilot CLI binary that is bundled via @github/copilot-sdk optional dependencies, while retaining a system-CLI fallback and surfacing CLI version/source in the UI.
Changes:
- Added a CLI locator utility that searches for bundled Copilot CLI binaries and falls back to PATH lookup.
- Updated Copilot SDK client initialization to pass an explicit
cliPath(with improved troubleshooting errors). - Displayed CLI version/source on the home screen and updated README prerequisites/troubleshooting.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/cli-locator.ts | New locator for bundled/system Copilot CLI and helper utilities. |
| src/utils/cli-locator.test.ts | Adds basic tests for CLI location behavior. |
| src/services/copilot.ts | Initializes SDK client using discovered cliPath and caches CLI info. |
| src/screens/home.tsx | Shows `[bundled |
| README.md | Removes manual CLI install prerequisite; adds troubleshooting and feature notes. |
| } catch (err) { | ||
| const message = (err as Error).message || 'Unknown error'; | ||
| throw new Error( | ||
| `Failed to start GitHub Copilot CLI.\n\n` + | ||
| `Error: ${message}\n\n` + | ||
| `The CLI was found at: ${cliLocation.path}\n` + | ||
| `Version: ${cliLocation.version}\n` + | ||
| `Source: ${cliLocation.source}\n\n` + | ||
| `Please ensure you have:\n` + | ||
| ` 1. Authenticated with GitHub Copilot (the CLI will prompt you)\n` + | ||
| ` 2. Active GitHub Copilot subscription\n` + | ||
| ` 3. Proper permissions to execute the CLI binary` | ||
| ); |
There was a problem hiding this comment.
The rethrown error from c.start() drops the original error object/stack, which makes diagnosing CLI startup failures harder. Consider preserving the original error as cause (or otherwise propagating the original stack) while still adding the extra troubleshooting context.
There was a problem hiding this comment.
Fixed in eaa681b - now using { cause: err } to preserve the original error object and stack trace while adding troubleshooting context.
src/utils/cli-locator.ts
Outdated
| }); | ||
| const path = result.trim().split('\n')[0]; | ||
|
|
||
| if (path && existsSync(path)) { | ||
| return path; | ||
| } | ||
| } catch { | ||
| // Ignore errors - CLI not in PATH |
There was a problem hiding this comment.
findSystemCli() runs execFileSync(which/where, ...) without a timeout. If PATH lookup hangs (network-mounted PATH entries, misconfigured shell hooks, etc.), Planeteer can block indefinitely during startup. Consider adding a short timeout and handling timeout errors explicitly.
| }); | |
| const path = result.trim().split('\n')[0]; | |
| if (path && existsSync(path)) { | |
| return path; | |
| } | |
| } catch { | |
| // Ignore errors - CLI not in PATH | |
| // Prevent PATH lookups from hanging indefinitely | |
| timeout: 2000, | |
| }); | |
| const path = result.trim().split('\n')[0]; | |
| if (path && existsSync(path)) { | |
| return path; | |
| } | |
| } catch (error) { | |
| // On timeout or lookup errors, treat as "CLI not in PATH" | |
| return null; |
There was a problem hiding this comment.
Fixed in eaa681b - added 2-second timeout to prevent indefinite blocking on PATH lookups from network-mounted entries or misconfigured shell hooks.
src/utils/cli-locator.test.ts
Outdated
| it('should locate a Copilot CLI binary', () => { | ||
| const location = locateCopilotCli(); | ||
|
|
||
| // Should find either bundled or system CLI | ||
| expect(location).toBeTruthy(); | ||
|
|
||
| if (location) { | ||
| expect(location.path).toBeTruthy(); | ||
| expect(['bundled', 'system']).toContain(location.source); | ||
| expect(location.version).toBeTruthy(); | ||
| } |
There was a problem hiding this comment.
These tests assume a Copilot CLI binary is present on the machine running the test (expect(location).toBeTruthy()), which makes CI and contributor runs brittle (e.g., when optional deps are omitted, unsupported platform/arch, or --ignore-scripts/--omit=optional). Prefer mocking existsSync/execFileSync to unit test the resolution logic, or relax the assertion to only validate the shape when location is non-null.
There was a problem hiding this comment.
Fixed in eaa681b - tests now handle null returns gracefully and only validate structure when CLI is found, making them CI-friendly for --omit=optional scenarios.
src/screens/home.tsx
Outdated
| // Load CLI info asynchronously | ||
| setCliInfo(getCliInfo()); |
There was a problem hiding this comment.
This comment says "Load CLI info asynchronously", but getCliInfo() is synchronous and can perform blocking work (process execution) under the hood. Either update the comment to match reality or move the CLI lookup off the render path (e.g., defer via setTimeout/Promise.resolve().then(...) and then setCliInfo).
| // Load CLI info asynchronously | |
| setCliInfo(getCliInfo()); | |
| // Load CLI info asynchronously (off the initial render path) | |
| Promise.resolve().then(() => { | |
| setCliInfo(getCliInfo()); | |
| }); |
There was a problem hiding this comment.
Fixed in eaa681b - moved CLI lookup off the render path using Promise.resolve().then() to defer the blocking work asynchronously.
|
@copilot apply changes based on the comments in this thread |
…sts, defer CLI lookup Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
✅ Implementation Complete: Bundle GitHub Copilot CLI
Overview
Successfully implemented zero-config GitHub Copilot CLI bundling for Planeteer. Users can now install and run Planeteer without manually installing the CLI.
Recent Updates (PR Review Feedback)
causewhen rethrowing CLI startup errorsAll Features Complete
src/services/copilot.tsto use bundled CLI with fallback to system CLIKey Features
✅ Zero-config installation: npm install && npm start works out of the box
✅ Bundled CLI v0.0.403: Automatically uses platform-specific binary
✅ Smart fallback: Falls back to system CLI if needed
✅ Version display: Shows CLI version and source on home screen
✅ Security hardened: No command injection, timeout protection
✅ CI-friendly: Tests work with --omit=optional
✅ Error diagnostics: Preserves original error stack for debugging
Security & Quality
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.