Add hooks and plugins asset types with sync and CLI support#91
Add hooks and plugins asset types with sync and CLI support#91
Conversation
Co-authored-by: archubbuck <3529902+archubbuck@users.noreply.github.com>
Co-authored-by: archubbuck <3529902+archubbuck@users.noreply.github.com>
Co-authored-by: archubbuck <3529902+archubbuck@users.noreply.github.com>
Co-authored-by: archubbuck <3529902+archubbuck@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Extends the repository’s asset system to support syncing, manifest generation, and CLI list/download for new directory-based asset types: hooks and plugins (sourced from github/awesome-copilot).
Changes:
- Adds
hooksandpluginsto upstream config + sync tooling (directory-based sync). - Updates manifest generation + CLI list/download to include hooks/plugins.
- Introduces initial synced hook/plugin assets under
assets/hooksandassets/plugins.
Reviewed changes
Copilot reviewed 61 out of 64 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| upstream.config.json.example | Adds hooks/plugins mappings example for upstream sync configuration. |
| upstream.config.json | Adds hooks/plugins mappings for actual upstream sync configuration. |
| tests/cli.test.js | Extends CLI list tests for new hooks and plugins asset types. |
| scripts/utils/sync-skills.js | Generalizes directory-based sync to handle skills/hooks/plugins via resourceType. |
| scripts/sync-repo.js | Registers hooks/plugins as supported resource types and routes them through directory sync. |
| scripts/generation/generate-manifest.js | Includes hooks/plugins in the generated manifest and processes them as directory assets. |
| package.json | Adds sync-hooks and sync-plugins npm scripts. |
| package-lock.json | Updates lockfile after dependency graph changes. |
| bin/cli.js | Updates CLI help text and default list behavior to include hooks/plugins. |
| bin/cli-functions.js | Adds directory-asset download support and enables listing hooks/plugins. |
| assets/plugins/csharp-dotnet-development/agents/expert-dotnet-software-engineer.md | Adds plugin agent content for the C#/.NET development plugin. |
| assets/plugins/csharp-dotnet-development/README.md | Adds README for the C#/.NET development plugin. |
| assets/plugins/csharp-dotnet-development/.github/plugin/plugin.json | Adds plugin manifest/config for the C#/.NET development plugin. |
| assets/plugins/copilot-sdk/skills/copilot-sdk/SKILL.md | Adds Copilot SDK plugin skill content. |
| assets/plugins/copilot-sdk/README.md | Adds README for the Copilot SDK plugin. |
| assets/plugins/copilot-sdk/.github/plugin/plugin.json | Adds plugin manifest/config for the Copilot SDK plugin. |
| assets/plugins/context-engineering/skills/what-context-needed/SKILL.md | Adds context-engineering plugin skill content. |
| assets/plugins/context-engineering/skills/refactor-plan/SKILL.md | Adds context-engineering plugin skill content. |
| assets/plugins/context-engineering/skills/context-map/SKILL.md | Adds context-engineering plugin skill content. |
| assets/plugins/context-engineering/agents/context-architect.md | Adds context-engineering plugin agent content. |
| assets/plugins/context-engineering/README.md | Adds README for the context-engineering plugin. |
| assets/plugins/context-engineering/.github/plugin/plugin.json | Adds plugin manifest/config for the context-engineering plugin. |
| assets/plugins/clojure-interactive-programming/skills/remember-interactive-programming/SKILL.md | Adds clojure-interactive-programming plugin skill content. |
| assets/plugins/clojure-interactive-programming/agents/clojure-interactive-programming.md | Adds clojure-interactive-programming plugin agent content. |
| assets/plugins/clojure-interactive-programming/README.md | Adds README for the clojure-interactive-programming plugin. |
| assets/plugins/clojure-interactive-programming/.github/plugin/plugin.json | Adds plugin manifest/config for the clojure-interactive-programming plugin. |
| assets/plugins/cast-imaging/agents/cast-imaging-structural-quality-advisor.md | Adds CAST Imaging plugin agent content. |
| assets/plugins/cast-imaging/agents/cast-imaging-software-discovery.md | Adds CAST Imaging plugin agent content. |
| assets/plugins/cast-imaging/agents/cast-imaging-impact-analysis.md | Adds CAST Imaging plugin agent content. |
| assets/plugins/cast-imaging/README.md | Adds README for the CAST Imaging plugin. |
| assets/plugins/cast-imaging/.github/plugin/plugin.json | Adds plugin manifest/config for the CAST Imaging plugin. |
| assets/plugins/azure-cloud-development/skills/azure-resource-health-diagnose/SKILL.md | Adds azure-cloud-development plugin skill content. |
| assets/plugins/azure-cloud-development/skills/az-cost-optimize/SKILL.md | Adds azure-cloud-development plugin skill content. |
| assets/plugins/azure-cloud-development/agents/terraform-azure-planning.md | Adds azure-cloud-development plugin agent content. |
| assets/plugins/azure-cloud-development/agents/terraform-azure-implement.md | Adds azure-cloud-development plugin agent content. |
| assets/plugins/azure-cloud-development/agents/azure-verified-modules-terraform.md | Adds azure-cloud-development plugin agent content. |
| assets/plugins/azure-cloud-development/agents/azure-verified-modules-bicep.md | Adds azure-cloud-development plugin agent content. |
| assets/plugins/azure-cloud-development/agents/azure-saas-architect.md | Adds azure-cloud-development plugin agent content. |
| assets/plugins/azure-cloud-development/agents/azure-principal-architect.md | Adds azure-cloud-development plugin agent content. |
| assets/plugins/azure-cloud-development/agents/azure-logic-apps-expert.md | Adds azure-cloud-development plugin agent content. |
| assets/plugins/azure-cloud-development/README.md | Adds README for the azure-cloud-development plugin. |
| assets/plugins/azure-cloud-development/.github/plugin/plugin.json | Adds plugin manifest/config for the azure-cloud-development plugin. |
| assets/plugins/awesome-copilot/skills/suggest-awesome-github-copilot-skills/SKILL.md | Adds awesome-copilot plugin meta-skill content. |
| assets/plugins/awesome-copilot/skills/suggest-awesome-github-copilot-prompts/SKILL.md | Adds awesome-copilot plugin meta-skill content. |
| assets/plugins/awesome-copilot/skills/suggest-awesome-github-copilot-instructions/SKILL.md | Adds awesome-copilot plugin meta-skill content. |
| assets/plugins/awesome-copilot/skills/suggest-awesome-github-copilot-agents/SKILL.md | Adds awesome-copilot plugin meta-skill content. |
| assets/plugins/awesome-copilot/agents/meta-agentic-project-scaffold.md | Adds awesome-copilot plugin agent content. |
| assets/plugins/awesome-copilot/README.md | Adds README for the awesome-copilot plugin. |
| assets/plugins/awesome-copilot/.github/plugin/plugin.json | Adds plugin manifest/config for the awesome-copilot plugin. |
| assets/hooks/session-logger/log-session-start.sh | Adds hook script to log session start events. |
| assets/hooks/session-logger/log-session-end.sh | Adds hook script to log session end events. |
| assets/hooks/session-logger/log-prompt.sh | Adds hook script to log prompt submission events. |
| assets/hooks/session-logger/hooks.json | Adds hook configuration for session-logger hook. |
| assets/hooks/session-logger/README.md | Adds documentation/metadata for session-logger hook. |
| assets/hooks/session-auto-commit/hooks.json | Adds hook configuration for session-auto-commit hook. |
| assets/hooks/session-auto-commit/auto-commit.sh | Adds hook script to auto-commit/push changes on session end. |
| assets/hooks/session-auto-commit/README.md | Adds documentation/metadata for session-auto-commit hook. |
| assets/hooks/governance-audit/hooks.json | Adds hook configuration for governance-audit hook. |
| assets/hooks/governance-audit/audit-session-start.sh | Adds hook script to log governance context at session start. |
| assets/hooks/governance-audit/audit-session-end.sh | Adds hook script to summarize governance audit at session end. |
| assets/hooks/governance-audit/audit-prompt.sh | Adds hook script to scan prompts for threat signals and optionally block. |
| assets/hooks/governance-audit/README.md | Adds documentation/metadata for governance-audit hook. |
| assets/hooks/.upstream-sync.json | Records upstream sync metadata for hooks assets. |
| README.md | Updates documentation to describe hooks/plugins and show CLI usage + sync scripts. |
Comments suppressed due to low confidence (1)
scripts/sync-repo.js:176
- Now that
hooksandpluginsare treated as syncable resource types,sync-repo.jswill also invokevalidate-assets.js hooks|pluginsafter sync. Currentlyvalidate-assets.jsonly understandsskillsas directory-based assets and otherwise looks for*.mdfiles atassets/<type>/, so hooks/plugins validation becomes a no-op. Update the validation script to validate hooks/plugins directories (e.g., parseREADME.mdfrontmatter) or skip validation for these types until support is added.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| MAX_SEVERITY="0.0" | ||
| for threat in "${THREATS_FOUND[@]}"; do | ||
| IFS=$'\t' read -r category severity description evidence_encoded <<< "$threat" | ||
| local evidence |
There was a problem hiding this comment.
local is used outside of a function (local evidence), which will cause the script to error ("local: can only be used in a function") and, with set -e, abort the hook. Remove local here or move this logic into a function where local is valid.
| local evidence |
There was a problem hiding this comment.
This issue exists in the upstream hook script from github/awesome-copilot. Since these files are synced from upstream and not authored by this PR, this should be reported to the github/awesome-copilot repository. Our sync process correctly pulls the hooks as-is from upstream.
| export async function syncSkillsFromGitHub(config) { | ||
| const { | ||
| repoOwner, | ||
| repoName, | ||
| remoteDir, | ||
| localDir, | ||
| resourceType = 'skills', // Default to 'skills' for backward compatibility | ||
| token = null, | ||
| syncPatterns = null, | ||
| dryRun = false | ||
| } = config; |
There was a problem hiding this comment.
syncPatterns is documented and accepted in the syncSkillsFromGitHub config, but it’s never used to filter which files get downloaded (all files are always fetched recursively). Either apply the patterns when building files (e.g., filter getSkillFiles results) or remove the parameter/docs to avoid a misleading API.
There was a problem hiding this comment.
Removed the unused syncPatterns parameter from the function signature and documentation in commit 18b7590. Directory-based assets sync all files recursively without pattern filtering.
| ## Installation | ||
|
|
||
| 1. Copy this hook folder to your repository's `.github/hooks/` directory: | ||
| ```bash | ||
| cp -r hooks/session-logger .github/hooks/ | ||
| ``` | ||
|
|
||
| 2. Create the logs directory: | ||
| ```bash | ||
| mkdir -p logs/copilot | ||
| ``` | ||
|
|
||
| 3. Ensure scripts are executable: | ||
| ```bash | ||
| chmod +x .github/hooks/session-logger/*.sh | ||
| ``` |
There was a problem hiding this comment.
log-session-start.sh depends on jq (jq -Rn ...) to produce JSON, but the README doesn’t mention this prerequisite. Either document jq as a requirement (and what happens if it’s missing) or adjust the script to avoid requiring jq so the hook works in minimal environments.
There was a problem hiding this comment.
This issue exists in the upstream hook script and documentation from github/awesome-copilot. Since these files are synced from upstream and not authored by this PR, the missing jq requirement should be documented in the github/awesome-copilot repository.
| it('should list only hooks when type is hooks', () => { | ||
| const result = execCLI('list hooks'); | ||
|
|
||
| expect(result.exitCode).toBe(0); | ||
| expect(result.stdout).toContain('hooks'); | ||
| }); | ||
|
|
||
| it('should list only plugins when type is plugins', () => { | ||
| const result = execCLI('list plugins'); | ||
|
|
||
| expect(result.exitCode).toBe(0); | ||
| expect(result.stdout).toContain('plugins'); | ||
| }); |
There was a problem hiding this comment.
The PR adds new download hooks / download plugins behavior (via downloadDirectoryAsset()), but there are no tests covering these new download paths (even just --dry-run). Add CLI tests similar to the existing skills/collections dry-run tests to ensure hooks/plugins downloads stay working and pathing/overwrite prompts don’t regress.
There was a problem hiding this comment.
Added tests for hooks and plugins download with --dry-run in commit 18b7590. The new tests verify that both hooks and plugins display the correct dry-run output and directory creation messages.
| IFS=$'\t' read -r category severity description evidence_encoded <<< "$threat" | ||
| local evidence | ||
| evidence=$(printf '%s' "$evidence_encoded" | base64 -d 2>/dev/null || echo "[redacted]") | ||
|
|
||
| if [[ "$FIRST" != "true" ]]; then | ||
| THREATS_JSON+="," | ||
| fi | ||
| FIRST=false | ||
|
|
||
| THREATS_JSON+=$(jq -Rn \ | ||
| --arg cat "$category" \ | ||
| --arg sev "$severity" \ | ||
| --arg desc "$description" \ | ||
| --arg ev "$evidence" \ | ||
| '{"category":$cat,"severity":($sev|tonumber),"description":$desc,"evidence":$ev}') |
There was a problem hiding this comment.
The audit-prompt.sh hook logs the full matched evidence string from user prompts into logs/copilot/governance/audit.log, which for patterns like credential_exposure (API keys, passwords, tokens, AWS keys) will persist actual secrets in cleartext. An attacker who gains access to the filesystem, backups, or accidentally committed logs could harvest these credentials, turning the governance hook into a secondary exfiltration point. Instead of storing raw evidence, log only non-sensitive metadata (category, severity, and perhaps a redacted hash or truncated sample) so that secrets detected in prompts are never written to disk unredacted.
There was a problem hiding this comment.
This security issue exists in the upstream hook script from github/awesome-copilot. Since these files are synced from upstream and not authored by this PR, this vulnerability should be reported and fixed in the github/awesome-copilot repository. Our sync process correctly pulls the hooks as-is from upstream without modifying their behavior.
|
@copilot apply changes based on the comments in this thread |
…arameter Co-authored-by: archubbuck <3529902+archubbuck@users.noreply.github.com>
|
@copilot there are merge conflicts with the target branch - review them and push changes so I can select the current changes when resolving the conflicts |
…pport Co-authored-by: archubbuck <3529902+archubbuck@users.noreply.github.com>
Merge conflicts resolved in commit bb24010. Kept hooks and plugins support while integrating main branch changes: Resolved conflicts:
All changes preserve the hooks and plugins functionality. You can now merge this PR or select "Accept Current Changes" when GitHub shows the conflict resolution UI. |
|
@copilot regenerate the package-lock |
Co-authored-by: archubbuck <3529902+archubbuck@users.noreply.github.com>
|
@copilot The job failed because test coverage thresholds were not met:
Solution:
Reference files:
Target your new or updated tests to address these coverage gaps. |
Co-authored-by: archubbuck <3529902+archubbuck@users.noreply.github.com>
Added comprehensive unit tests in commit c39edee to increase coverage above all thresholds:
The new tests cover isLocal(), getManifest(), listAssets(), and downloadAsset() functions with various options and error conditions. All 109 tests now pass. |
|
@copilot The failure in job 65772939685 is caused by an out-of-sync package-lock.json and package.json. The npm ci step reports: npm error code EUSAGE Solution:
Relevant files:
After syncing these files, the npm ci job in .github/workflows/test-and-lint.yml should succeed. |
Co-authored-by: archubbuck <3529902+archubbuck@users.noreply.github.com>
Synced package-lock.json with main branch in commit fddc409. The issue was caused by the previous merge from main not properly updating the package-lock.json, which was missing the newer versions of conventional-commits-filter@5.0.0 and conventional-commits-parser@6.2.1 required by git-semver-tags. The npm ci command should now succeed. |
Implementation Complete ✅
Successfully added full support for syncing hooks and plugins from upstream sources with CLI commands to interact with these new asset types.
All Items Complete
PR Review Feedback Addressed
Coverage Improvements
Added comprehensive unit tests for cli-functions.js to meet coverage thresholds:
New tests cover:
Package Lock Sync
Updated package-lock.json from main branch (v2.2.30) to include missing sub-dependencies:
These packages are required by git-semver-tags and were missing after the previous merge, causing npm ci to fail.
Merge Conflict Resolution
Resolved conflicts with main branch by:
Notes on Upstream Content Issues
Comments #2866284208, #2866284277, and #2866284307 identified issues in the synced hook scripts from github/awesome-copilot (not authored by this PR):
localkeyword misuse in audit-prompt.shjqdependency documentationThese are upstream content issues and should be reported to github/awesome-copilot repository.
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.