Skip to content

fix: skills export path + init workflow filtering - #847

Merged
bradygaster merged 1 commit into
devfrom
squad/fix-insider-remaining-tests
Apr 5, 2026
Merged

fix: skills export path + init workflow filtering#847
bradygaster merged 1 commit into
devfrom
squad/fix-insider-remaining-tests

Conversation

@bradygaster

Copy link
Copy Markdown
Owner

Fixes the last 7 test failures blocking the insider release. Skills export now checks .copilot/skills/ in addition to .ai-team/skills/. Init no longer copies CI/CD workflows (those are upgrade-only).

- Export now reads skills from both .ai-team/skills/ and .copilot/skills/
- Init no longer copies CI/CD workflows (squad-ci, squad-release, etc.)
  to .github/workflows/ — those are upgrade-only

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 5, 2026 09:04
@github-actions

github-actions Bot commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

🟢 Impact Analysis — PR #847

Risk tier: 🟢 LOW

📊 Summary

Metric Count
Files changed 1
Files added 0
Files modified 1
Files deleted 0
Modules touched 1

🎯 Risk Factors

  • 1 files changed (≤5 → LOW)
  • 1 module(s) touched (≤1 → LOW)

📦 Modules Affected

root (1 file)
  • index.cjs

This report is generated automatically for every PR. See #733 for details.

@github-actions

github-actions Bot commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit dc5ab57

⚠️ 2 item(s) to address before review

Status Check Details
Single commit 1 commit — clean history
Not in draft Ready for review
Branch up to date Up to date with dev
Copilot review No Copilot review yet — it may still be processing
Changeset present No source files changed — changeset not required
Scope clean No .squad/ or docs/proposals/ files
No merge conflicts No merge conflicts
Copilot threads resolved No Copilot review threads
CI passing 13 check(s) still running

This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 addresses remaining test failures blocking the insider release by updating the export/import support for skills and tightening init behavior around workflow installation.

Changes:

  • Export now reads skills from .copilot/skills/ as well as legacy .ai-team/skills/.
  • init filters workflow templates so CI/CD workflows are not installed until upgrade.

Comment thread index.cjs
Comment on lines +1065 to +1069
const skillsDirs = [skillsDir, copilotSkillsDir].filter(d => fs.existsSync(d));
try {
if (fs.existsSync(skillsDir)) {
for (const entry of fs.readdirSync(skillsDir)) {
const skillFile = path.join(skillsDir, entry, 'SKILL.md');
for (const sDir of skillsDirs) {
for (const entry of fs.readdirSync(sDir)) {
const skillFile = path.join(sDir, entry, 'SKILL.md');

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exporting skills from both .ai-team/skills and .copilot/skills can produce duplicates when a repo still has the legacy directory alongside the new one. On import, skills with the same name: (or identical content) will silently overwrite each other because they’re written to the same .ai-team/skills/<skillName>/SKILL.md path. Consider treating .copilot/skills as the canonical source when present (fallback to .ai-team/skills only if it’s absent), or de-duplicating by skill directory name / parsed name: so each exported skill is unique and stable.

Copilot uses AI. Check for mistakes.
Comment thread index.cjs
Comment on lines +1839 to +1843
// During init, only copy framework workflows — CI/CD workflows are installed by upgrade
const initWorkflows = workflowFiles.filter(f => !PROJECT_TYPE_SENSITIVE_WORKFLOWS.has(f));
fs.mkdirSync(workflowsDest, { recursive: true });
let copied = 0;
for (const file of workflowFiles) {
for (const file of initWorkflows) {

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Init now filters out CI/CD workflows via initWorkflows, but the later “all squad workflows already exist — skipping” condition/message still keys off workflowFiles.length. On a re-run of init, this message becomes misleading because the filtered CI/CD workflows won’t exist by design. Consider basing the condition/message on initWorkflows (e.g., “all init workflows already exist”) so the output matches the new behavior.

Copilot uses AI. Check for mistakes.
@bradygaster
bradygaster merged commit f522840 into dev Apr 5, 2026
22 checks passed
@bradygaster
bradygaster deleted the squad/fix-insider-remaining-tests branch April 5, 2026 09:09
tamirdresher added a commit that referenced this pull request Apr 23, 2026
The 3 dotnet project type tests (.slnx, .fsproj, .vbproj) expected
squad-ci.yml to be created during init. Since PR #847 init intentionally
skips CI/CD workflows (they are installed by upgrade). Change the tests
to run init first (creates .squad/ structure) then upgrade (installs
CI workflows including project-type-specific ones).

Fixes the last 3 test failures blocking the 0.9.4 release.

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tamirdresher pushed a commit that referenced this pull request Apr 23, 2026
Same fix as PR #1030 on dev. Changes 3 tests in Group 6 from init-only
to init+upgrade pattern, matching the behavior change from PR #847.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants