test: add install verification workflow#922
Conversation
Co-authored-by: rjmurillo <6811113+rjmurillo@users.noreply.github.com>
PR Validation ReportNote ✅ Status: PASS Description Validation
QA Validation
⚡ Warnings
Powered by PR Validation workflow |
Session Protocol Compliance ReportTip ✅ Overall Verdict: PASS All session protocol requirements satisfied. What is Session Protocol?Session logs document agent work sessions and must comply with RFC 2119 requirements:
See .agents/SESSION-PROTOCOL.md for full specification. Compliance Summary
Detailed Validation ResultsClick each session to see the complete validation report with specific requirement failures. 📄 sessions-2026-01-15-session-2-add-workflow-verify-install-script✨ Zero-Token ValidationThis validation uses deterministic PowerShell script analysis instead of AI:
Powered by Validate-SessionJson.ps1 📊 Run Details
Powered by Session Protocol Validator workflow |
AI Quality Gate ReviewTip ✅ Final Verdict: PASS WalkthroughThis PR was reviewed by six AI agents in parallel, analyzing different aspects of the changes:
Review Summary
💡 Quick Access: Click on individual agent jobs (e.g., "🔒 security Review", "🧪 qa Review") in the workflow run to see detailed findings and step summaries. Security Review DetailsNow let me view the session JSON files briefly to check for any exposed secrets: Now let me check the workflow file more carefully for injection vulnerabilities and permissions: Security Review AnalysisPR Category Assessment
Findings
Analysis DetailsWorkflow Security (verify-install-script.yml)
PowerShell Script Security (install.ps1)
Verification Script Security (Verify-InstallOutput.ps1)
Test File Security (Verify-InstallOutput.Tests.ps1)
Session JSON Files
RecommendationsNone required. The PR follows security best practices. QA Review DetailsBased on my analysis, I can now provide the QA review: QA Review: PR #922 - test: add install verification workflowTest Coverage Assessment
Quality Concerns
Regression Risk Assessment
Evidence SummaryStrengths:
Test Coverage Verification:
Error Handling:
Analyst Review DetailsLet me examine the existing codebase patterns and the key files to complete the analysis. Based on my analysis of the PR diff, existing codebase patterns, and workflow structures, here is my complete assessment: Code Quality Score
Overall: 4.25/5 Impact Assessment
Findings
Recommendations
VerdictSummary of positive elements:
Architect Review DetailsBased on my review of the PR changes, I can now provide a complete architectural assessment. Design Quality Assessment
Overall Design Score: 4.8/5 Architectural Concerns
Breaking Change Assessment
Technical Debt Analysis
ADR Assessment
Rationale: This PR implements existing architectural decisions rather than introducing new ones. The file-based verification approach was already documented in the issue research and follows established CI patterns. Recommendations
VerdictDevOps Review DetailsLet me also check the install.ps1 changes and docs update: Now I have all the context needed to provide a comprehensive DevOps review. DevOps Review: PR #922 - Install Verification WorkflowPR Scope Detection
Review scope: Full CI/CD review for workflow, shell quality review for scripts. Pipeline Impact Assessment
CI/CD Quality Checks
Findings
Detailed Analysis1. Workflow Structure (Excellent)
2. Action Pinning (Pass)
3. Permissions (Pass)
4. Shell Script Quality (Pass)
5. Cross-Platform Handling (Pass)
6. Pester Tests (Pass)
Template Assessment
Automation Opportunities
Recommendations
VerdictRoadmap Review DetailsI have sufficient context to provide the roadmap review analysis. Strategic Alignment Assessment
Feature Completeness
Impact Analysis
Concerns
Recommendations
VerdictRun Details
Powered by AI Quality Gate workflow |
When install.ps1 was run in Claude Repo scope, the Skills installation attempted to copy .claude/skills to .claude/skills (same source and destination), causing an infinite recursive copy operation. The fix adds: - Path normalization using [System.IO.Path]::GetFullPath() for both source and destination paths - Same-path detection that compares normalized paths - Graceful handling when source equals destination with informative message: "Skills already present in repository" - Fallback message when source directory doesn't exist This ensures the script works correctly across all installation scopes (Global, Repo) without attempting self-referential copy operations. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Updated workLog with push, PR title fix, and CI verification actions - Recorded all 18 matrix jobs passed - Updated endingCommit to ffa395b - Marked session end gates as complete Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive CI workflow to verify the PowerShell installation script (scripts/install.ps1) works correctly across all supported platforms (Windows, macOS, Linux) and environments (Claude, Copilot, VS Code) in both Global and Repo scopes. The verification uses file-based checks to validate installations without requiring CLI authentication, addressing a known limitation. A bug in the skills installation logic was also fixed during CI testing.
Changes:
- Added GitHub Actions workflow with 18-job matrix (3 OS × 3 environments × 2 scopes) using paths-filter optimization
- Created verification helper script with comprehensive file/directory validation logic
- Fixed pre-existing bug where Claude Repo scope attempted self-referential skills copy
- Added Pester tests for verification script structure and documentation updates
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/verify-install-script.yml |
CI workflow with matrix strategy, paths-filter, and APPDATA setup for non-Windows runners |
scripts/tests/Verify-InstallOutput.ps1 |
Verification helper that validates installed files/directories against config expectations |
scripts/tests/Verify-InstallOutput.Tests.ps1 |
Pester tests covering script structure, parameters, and module integration |
scripts/install.ps1 |
Fixed skills installation bug with path normalization and same-path detection |
scripts/README.md |
Added CI verification section with local usage examples |
.serena/memories/install-script-ci-verification-workflow.md |
Serena memory documenting the implementation |
.agents/sessions/2026-01-15-session-2-add-workflow-verify-install-script.json |
Session log for initial implementation |
.agents/sessions/2026-01-15-session-03.json |
Session log documenting bug fix and CI completion |
| } | ||
| } | ||
|
|
||
| if ($Config.InstructionsFile -and $null -ne $Config.InstructionsDest) { |
There was a problem hiding this comment.
The null check $null -ne $Config.InstructionsDest is inconsistent with other checks in the script. Lines 117, 129, and 143 use implicit truthiness checks without explicit null comparisons. For consistency, this should be $Config.InstructionsDest unless there's a specific reason to distinguish between null and empty string.
| if ($Config.InstructionsFile -and $null -ne $Config.InstructionsDest) { | |
| if ($Config.InstructionsFile -and $Config.InstructionsDest) { |
| } else { | ||
| Join-Path $env:HOME '.config' |
There was a problem hiding this comment.
The APPDATA setup for non-Windows platforms uses string comparison '${{ runner.os }}' -eq 'macOS'. While functional, this assumes Linux is the only other option. Consider using explicit checks for both macOS and Linux or adding a comment explaining the assumption that non-Windows, non-macOS means Linux.
| } else { | |
| Join-Path $env:HOME '.config' | |
| } elseif ('${{ runner.os }}' -eq 'Linux') { | |
| Join-Path $env:HOME '.config' | |
| } else { | |
| throw "Unsupported runner OS '${{ runner.os }}' for APPDATA configuration." |
| It "Uses Get-AgentFiles and Test-SourceDirectory" { | ||
| $Script:Content | Should -Match "Get-AgentFiles" | ||
| $Script:Content | Should -Match "Test-SourceDirectory" | ||
| } |
There was a problem hiding this comment.
The tests only verify that the verification script has the correct structure and contains references to key functions. However, there are no integration tests that actually run Verify-InstallOutput.ps1 with mock installations to validate it correctly identifies missing files or directories. Consider adding integration tests that create temporary test installations and verify the script correctly passes/fails based on file presence.
Original prompt
This section details on the original issue you should resolve
<issue_title>test: add CI workflow to verify install script on Windows, macOS, and Linux</issue_title>
<issue_description>## Summary
Create a GitHub Actions workflow to verify the PowerShell install script (
scripts/install.ps1) works correctly on all supported platforms.Background
The install script installs agent definitions to three environments:
$HOME/.claude/agents/(global) or.claude/agents/(repo)$HOME/.config/gh-copilot/agents/(global) or.copilot/agents/(repo)$HOME/.vscode/agents/(global) or.vscode/agents/(repo)Currently, the install script is only tested manually. Automated CI verification ensures cross-platform compatibility.
Research Findings
Authentication Blockers
Recommendation: Use file-based verification (check files exist and match source) rather than CLI verification. This tests 90% of script functionality without authentication.
PowerShell Availability
PowerShell Core (
pwsh) is preinstalled on all GitHub-hosted runners:ubuntu-latest: pwsh 7.xmacos-latest: pwsh 7.xwindows-latest: pwsh 7.x (plus Windows PowerShell 5.1)CLI Installation Commands (for reference)
Claude Code CLI:
GitHub Copilot CLI:
# Via gh extension gh extension install github/gh-copilotProposed Workflow Design
Option A: Single Workflow with Matrix (Recommended)
Option B: Separate Workflows per OS
Pros: OS-specific configuration easier to manage
Cons: More files to maintain, harder to see overall status
Acceptance Criteria
MUST Requirements
scripts/install.ps1orscripts/lib/**changeSHOULD Requirements
dorny/paths-filterto skip when no relevant files changedpester-tests.ymlpatternsscripts/tests/Verify-InstallOutput.ps1WILL NOT (Out of Scope)
iexfrom GitHub URL)Implementation Tasks
Verify-InstallOutput.ps1helper script.github/workflows/verify-install-script.ymlscripts/README.mdCost Estimate
Related
scripts/install.ps1scripts/lib/Install-Common.psm1scripts/lib/Config.psd1scripts/tests/Analysis artifact:
.agents/analysis/install-script-ci-workflow-analysis.md</issue_description><agent_instructions>## Observations
The repository has a well-established PowerShell-based installation system with
file:scripts/install.ps1as the unified entry point,file:scripts/lib/Install-Common.psm1providing shared functions, andfile:scripts/lib/Config.psd1defining environment-specific paths. The existingfile:.github/workflows/pester-tests.ymldemonstrates the pattern for cross-platform testing with `dorny/paths...💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.