Implement Copilot SDK integration and multi-convention commit system #41
Conversation
Co-authored-by: warengonzaga <15052701+warengonzaga@users.noreply.github.com>
Co-authored-by: warengonzaga <15052701+warengonzaga@users.noreply.github.com>
Co-authored-by: warengonzaga <15052701+warengonzaga@users.noreply.github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the
Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements GitHub Copilot SDK integration and introduces a multi-convention commit message system with four supported formats: Clean Commit (default), Conventional Commits, Gitmoji, and Simple. The implementation replaces the placeholder Copilot integration with real SDK calls and adds interactive convention switching during the commit process.
Changes:
- Adds pluggable convention system with four pre-configured formats and AI-specific prompts for each
- Implements GitHub Copilot SDK client lifecycle management with automatic OpenAI fallback
- Adds interactive convention switching capability with regeneration loops
- Updates authentication to use GitHub CLI instead of token-based auth
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| source/utils/commit-conventions.js | New file defining four commit conventions with format-specific prompts |
| source/providers/ai-provider.js | Replaces placeholder with real Copilot SDK integration (CopilotClient lifecycle) |
| source/commands/commit.js | Adds regeneration loops with convention switching for file-by-file and single-file workflows |
| source/commands/auth.js | Removes token parameter, validates gh CLI installation and authentication status |
| source/commands/config.js | Adds convention name display in config output |
| source/cli.js | Adds --convention flag and set-convention subcommand with validation |
| source/utils/ui.js | Adds promptConventionSelection and updates confirmCommit to show convention and regeneration option |
| source/utils/config-manager.js | Adds setConvention/getConvention functions with 'clean' default |
| test.js | Adds comprehensive tests for convention system and config management |
Comments suppressed due to low confidence (1)
source/cli.js:64
- Superfluous argument passed to function authenticateWithCopilot.
authenticateWithCopilot(options.token);
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: warengonzaga <15052701+warengonzaga@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (1)
source/cli.js:65
- Superfluous argument passed to function authenticateWithCopilot.
authenticateWithCopilot(options.token);
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: warengonzaga <15052701+warengonzaga@users.noreply.github.com>
Co-authored-by: warengonzaga <15052701+warengonzaga@users.noreply.github.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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
source/cli.js (1)
63-66:⚠️ Potential issue | 🟡 MinorRemove unused
--tokenoption.The
--tokenoption is passed toauthenticateWithCopilot()but the function no longer accepts parameters (seesource/commands/auth.jsline 22).🔧 Remove the dead code
authCmd .command('copilot') .description('Authenticate with GitHub Copilot') - .option('--token <token>', 'GitHub token') - .action(options => { - authenticateWithCopilot(options.token); + .action(() => { + authenticateWithCopilot(); });source/commands/commit.js (1)
91-106:⚠️ Potential issue | 🟡 MinorConvention support missing in
processAllFiles.Unlike
processFilesInteractivelyandprocessFile, this function:
- Doesn't pass the convention to
generateCommitMessage(line 91)- Doesn't pass the convention to
confirmCommitfor display (line 94)- Doesn't support the regeneration flow when user selects "change-convention"
This means
--allusers won't see convention info or be able to regenerate with different conventions.🔧 Suggested fix to add convention support
export async function processAllFiles(aiProvider, options = {}) { showInfo('Processing all files together...'); // Stage all changes await stageAll(); // Get full diff const diff = await getDiff(); if (!diff) { showWarning('No changes to commit.'); return; } + let currentConvention = options.convention || getConvention(); + let attempts = 0; + const maxAttempts = 5; + + while (attempts < maxAttempts) { - // Generate commit message - showInfo('Generating commit message...'); - const message = await aiProvider.generateCommitMessage(diff, null, options); + showInfo('Generating commit message...'); + const message = await aiProvider.generateCommitMessage(diff, null, { + ...options, + convention: currentConvention, + }); - // Confirm and commit - const result = await confirmCommit(message); + const result = await confirmCommit(message, null, currentConvention); - if (result.action === 'accept') { - const success = await commit(result.message); - if (success) { - showSuccess('Changes committed successfully!'); - console.log(`📝 ${result.message}`); - } else { - showError('Failed to commit changes.'); + if (result.action === 'accept') { + const success = await commit(result.message); + if (success) { + showSuccess('Changes committed successfully!'); + console.log(`📝 ${result.message}`); + } else { + showError('Failed to commit changes.'); + } + return; + } else if (result.action === 'regenerate') { + currentConvention = result.convention; + attempts++; + continue; + } else if (result.action === 'skip') { + showInfo('Commit cancelled.'); + return; } - } else if (result.action === 'skip') { - showInfo('Commit cancelled.'); } + + showWarning('Maximum regeneration attempts reached.'); }
🤖 Fix all issues with AI agents
In `@source/commands/auth.js`:
- Around line 22-24: The CLI's --token option is unused because
authenticateWithCopilot() in source/commands/auth.js no longer accepts a token;
either remove the dead option from the CLI or update the function to accept and
use it — locate authenticateWithCopilot and the CLI registration of the "auth
copilot" command (options.token passed in source/cli.js) and do one of: (A)
delete the --token option from the command definition in source/cli.js and any
references to options.token, or (B) change authenticateWithCopilot to accept a
token parameter (e.g., authenticateWithCopilot(token) or an options object) and
thread options.token from the CLI into that function and use it where
authentication requires a token.
🧹 Nitpick comments (2)
source/utils/config-manager.js (1)
21-26: Consider logging or returning feedback when ignoring unsupported providers.
setTokensilently ignores any provider other than'openai'. While this aligns with the PR's goal to remove Copilot token handling, callers have no way to know if their token was stored. Consider either throwing an error for unsupported providers or returning a boolean to indicate success.♻️ Optional: Add feedback for unsupported providers
export function setToken(provider, token) { if (provider === 'openai') { config.set('openai', token); config.set('authenticatedAt', new Date().toISOString()); + return true; } + return false; }source/cli.js (1)
49-52: Consider validating--conventionoption at CLI level.The
--conventionoption is validated inset-conventionbut not when passed to thecommitcommand. While downstream code handles unknown conventions by falling back to'clean', users won't get immediate feedback about typos.♻️ Optional: Add validation for --convention option
.option( '--convention <type>', 'Commit convention (clean, conventional, gitmoji, simple)', ) .action(options => { + if (options.convention && !Object.keys(CONVENTIONS).includes(options.convention)) { + showError(`Invalid convention: ${options.convention}`); + console.log(`Available: ${Object.keys(CONVENTIONS).join(', ')}`); + process.exit(1); + } commitCommand(options); });
Co-authored-by: warengonzaga <15052701+warengonzaga@users.noreply.github.com>
Co-authored-by: warengonzaga <15052701+warengonzaga@users.noreply.github.com>
warengonzaga
merged commit ee2b819
into
copilot/migrate-to-commander-and-copilot
Implementation Plan: GitHub Copilot SDK Integration and Multi-Convention Support
Phase 1: Core Infrastructure
source/utils/commit-conventions.jswith 4 conventions (clean, conventional, gitmoji, simple)source/utils/config-manager.jsPhase 2: AI Provider Integration
source/providers/ai-provider.jswith real Copilot SDK integrationPhase 3: Authentication Updates
source/commands/auth.jsto check for GitHub CLIPhase 4: UI Enhancements
promptConventionSelection()tosource/utils/ui.jsconfirmCommit()to support convention switchingPhase 5: Command Updates
source/commands/commit.jswith regeneration supportPhase 6: CLI Interface
--conventionflag to commit command insource/cli.jsconfig set-conventionsubcommandPhase 7: Testing & Validation
Original prompt
Implement GitHub Copilot SDK Integration and Multi-Convention Support
Objective
Complete the GitHub Copilot SDK integration in PR #40 and add support for multiple commit message conventions with Clean Commit as the default.
Base Branch
This PR should target the
copilot/migrate-to-commander-and-copilotbranch from PR #40.Requirements
1. Create Commit Convention System
Create new file:
source/utils/commit-conventions.js