-
Notifications
You must be signed in to change notification settings - Fork 5
feat(copilot): enrich installer with addition of tool selection #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
634475c to
67d84b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for installing AWOS for different AI coding tools (Claude, Copilot, or both) via a new --tool flag. The implementation introduces tool-specific filtering for directories and copy operations, a new settings merger service for VS Code configuration, and creates Copilot-specific prompt and agent files.
Key Changes
- Added
--toolCLI argument with support forclaude,copilot, orallvalues - Introduced settings merger service to handle VS Code settings.json merging
- Created Copilot-specific prompts (.github/prompts/.prompt.md) and agents (.github/agents/.agent.md) that mirror the Claude workflow
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.js | Adds CLI argument parsing and validation for the --tool flag |
| src/core/setup-orchestrator.js | Integrates tool filtering and VS Code settings merge into setup workflow |
| src/config/setup-config.js | Adds tool-based filtering functions and metadata to directory/operation configs |
| src/services/settings-merger.js | New service providing JSON settings merge functionality with deep merge support |
| src/utils/logger.js | Extends summary display to include VS Code settings merge/create statistics |
| copilot/vscode/settings.json | Defines Copilot workspace settings for prompt recommendations and script auto-approval |
| copilot/prompts/*.prompt.md | Seven prompt files delegating to corresponding .awos/commands/*.md files |
| copilot/agents/*.agent.md | Three agent files delegating to corresponding .awos/subagents/*.md files |
- Added validation for the `--tool` argument to ensure it requires a value and is one of the supported tools. - Updated the `deepMerge` function to merge arrays by combining unique values, preserving user settings. - Improved error handling for JSON parsing in settings files, logging specific errors for invalid JSON. These changes aim to enhance user experience and ensure robust settings management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 13 comments.
| } catch (err) { | ||
| log(`Failed to inline ${relativePath}: ${err.message}`, 'error'); | ||
| stats.skipped++; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Write generated file | ||
| if (!dryRun) { | ||
| const outputPath = path.join(outputDir, file); | ||
| await fs.writeFile(outputPath, content, 'utf8'); | ||
| log(`Generated ${file}`, 'success'); | ||
| } | ||
|
|
||
| stats.generated++; |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an inline file fails to load, the function logs an error and increments skipped but then continues to increment generated count. This means a file that failed to inline properly will still be counted as successfully generated. The continue statement should skip to the next file in the outer loop, but it actually just skips to the next inline match. If any inline fails, the partially-processed file should not be written or counted as generated.
| if ( | ||
| source[key] && | ||
| typeof source[key] === 'object' && | ||
| !Array.isArray(source[key]) | ||
| ) { | ||
| result[key] = deepMerge(result[key] || {}, source[key]); |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deepMerge function doesn't handle null values in the source correctly. Line 24 checks source[key] && typeof source[key] === 'object', which will evaluate to false for null (since null is falsy). This means source[key] = null will fall through to the else branch at line 50, which overwrites the target value. However, if the intention is to allow null to overwrite target values, this is correct but should be documented. If not, null should be explicitly handled.
| // Add VS Code settings status for Copilot | ||
| if (statistics.settingsMerged) { | ||
| summaryItems.push(['VS Code settings merged', 1]); | ||
| } else if (statistics.settingsCreated) { | ||
| summaryItems.push(['VS Code settings created', 1]); | ||
| } |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The summary is missing display of prompts and agents generation statistics. The statistics object includes promptsGenerated and agentsGenerated (set in setup-orchestrator.js lines 141-142), but these are not being displayed in the summary. Users won't see how many prompts and agents were generated during Copilot installation.
| // Write generated file | ||
| if (!dryRun) { | ||
| const outputPath = path.join(outputDir, file); | ||
| await fs.writeFile(outputPath, content, 'utf8'); | ||
| log(`Generated ${file}`, 'success'); |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In dry-run mode, the function doesn't log what files would be generated. Users running with --dry-run won't see which prompt/agent files would be created. Consider adding dry-run logging similar to other operations in the codebase (e.g., line 112: "[DRY-RUN] Would merge settings").
| // Write generated file | |
| if (!dryRun) { | |
| const outputPath = path.join(outputDir, file); | |
| await fs.writeFile(outputPath, content, 'utf8'); | |
| log(`Generated ${file}`, 'success'); | |
| // Write generated file (or log in dry-run mode) | |
| const outputPath = path.join(outputDir, file); | |
| if (!dryRun) { | |
| await fs.writeFile(outputPath, content, 'utf8'); | |
| log(`Generated ${file}`, 'success'); | |
| } else { | |
| log(`[DRY-RUN] Would generate ${file} at ${outputPath}`, 'info'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/utils/logger.js:1
- The
promptsGeneratedstatistic is added to the statistics object in setup-orchestrator.js but is never displayed in the summary. The showSummary function needs to handle this statistic similar to how it handlessettingsMergedandsettingsCreated. Add logic to display 'Prompts generated' whenstatistics.promptsGenerated > 0.
/**
- prompt-generator: skip writing file entirely when any inline fails, preventing partial/corrupted output files - settings-merger: replace JSON.stringify comparison with proper deepEqual function for order-independent object comparison in array deduplication - settings-merger: fix misleading JSDoc (objects are merged, not overwritten) - setup-config: handle 'all' tool filter specially, removing redundant 'all' entries from every tools array Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use replaceAll() instead of replace() to handle multiple occurrences of the same inline marker - Support Windows line endings (\r\n) in frontmatter detection - Add dry-run logging to show which files would be generated - Clarify JSDoc: 'skipped' means files with failed inlines Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/utils/logger.js:72
- The JSDoc for
showSummaryis missing documentation for theoptions.toolparameter that is now being passed to this function in setup-orchestrator.js line 132.
* @param {Object} statistics - Statistics about the setup process
* @param {Object} options - Options for the setup process
* @param {boolean} options.dryRun - Whether this was a dry-run
| promptsGenerated: promptStatistics.generated, | ||
| }; | ||
| showSummary(statistics, { dryRun }); | ||
| showSummary(statistics, { dryRun, tool }); |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tool parameter is passed to showSummary but is not used in its implementation. Either use this parameter to customize the summary output based on tool selection, or remove it from the function call.
| showSummary(statistics, { dryRun, tool }); | |
| showSummary(statistics, { dryRun }); |
|
|
||
| for (const key of Object.keys(source)) { | ||
| if ( | ||
| source[key] && |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition source[key] && on line 52 will skip merging when source[key] is null, even though null is a valid JSON value that should overwrite target values. This check was likely intended to exclude null from object merging, but it prevents legitimate null values from being set. Consider checking for source[key] !== null explicitly instead.
| source[key] && | |
| source[key] !== null && |
| if (dryRun) { | ||
| log(`[DRY-RUN] Would generate: ${outputPath}`, 'info'); | ||
| } else { | ||
| await fs.writeFile(outputPath, content, 'utf8'); | ||
| log(`Generated ${file}`, 'success'); | ||
| } | ||
|
|
||
| stats.generated++; |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unlike the file-copier service which explicitly handles existing files (checking and unlinking before copy), this function always overwrites existing prompt files without checking. For consistency with the rest of the installer and to avoid surprising users, consider adding a check for existing files and either skip generation (preserving user modifications) or log that the file is being updated.
| if (dryRun) { | |
| log(`[DRY-RUN] Would generate: ${outputPath}`, 'info'); | |
| } else { | |
| await fs.writeFile(outputPath, content, 'utf8'); | |
| log(`Generated ${file}`, 'success'); | |
| } | |
| stats.generated++; | |
| // Check if output file already exists to avoid overwriting user changes | |
| let exists = false; | |
| try { | |
| await fs.access(outputPath); | |
| exists = true; | |
| } catch (err) { | |
| // File does not exist; safe to generate | |
| exists = false; | |
| } | |
| if (dryRun) { | |
| if (exists) { | |
| log(`[DRY-RUN] Would skip (already exists): ${outputPath}`, 'info'); | |
| stats.skipped++; | |
| } else { | |
| log(`[DRY-RUN] Would generate: ${outputPath}`, 'info'); | |
| stats.generated++; | |
| } | |
| } else { | |
| if (exists) { | |
| log(`Skipping ${file} (already exists)`, 'info'); | |
| stats.skipped++; | |
| } else { | |
| await fs.writeFile(outputPath, content, 'utf8'); | |
| log(`Generated ${file}`, 'success'); | |
| stats.generated++; | |
| } | |
| } |
Enrich the installer with the
--toolflag that allows to install either both Claude and Copilot Chat or only Claude/CopilotThis one is quite similar to #55, but it makes all more aligned with the current Agent support and splits AWOS installation similar to the Claude Code one
It installs properly, but I haven't tried to build something