fix: add onboarding roster guard and skill overwrite warning by diberry · Pull Request #867 · github/squad

diberry · 2026-04-05T19:34:48Z

Architecture Audit — 2 Medium-Priority Fixes

Item 1: Onboarding flow production guard

After createTeam() in finalizeCast(), verify the roster is populated before re-dispatching to the coordinator. If empty:

Cleans up .init-prompt (prevents auto-retry loops)
Shows user-visible warning
Clears activity hint and processing state

File: packages/squad-cli/src/cli/shell/index.ts

Item 2: Skill upgrade overwrite warning

During squad upgrade, warn when a customized built-in skill is about to be overwritten. Fires in both code paths:

Manifest upgrade loop (full version upgrade)
syncAllSkills() (already-current ensure checks)

Normalizes line endings before comparison to avoid false positives on Windows.

File: packages/squad-cli/src/cli/core/upgrade.ts

Item 3: Regression tests for finalizeCast roster guard

Added 7 regression tests in test/init-autocast.test.ts under a new finalizeCast roster guard — empty roster prevents dispatch describe block, covering:

Empty roster after createTeam() → .init-prompt deleted (prevents auto-retry loop)
Empty roster after createTeam() → dispatch aborted (early return)
Populated roster → .init-prompt preserved, dispatch proceeds normally
Edge cases: header-only table, absent .init-prompt, completely empty team.md

File: test/init-autocast.test.ts

Notes

Pre-existing build error in start.ts is unrelated
Both fixes are additive guards/warnings — no behavior changes
Regression tests ensure the empty-roster dispatch guard cannot be silently removed in future refactors
Changeset included

Requested by: Dina Berry

github-actions · 2026-04-05T19:35:02Z

🟡 Impact Analysis — PR #867

Risk tier: 🟡 MEDIUM

📊 Summary

Metric	Count
Files changed	5
Files added	1
Files modified	4
Files deleted	0
Modules touched	3
Critical files	1

🎯 Risk Factors

5 files changed (≤5 → LOW)
3 modules touched (2-4 → MEDIUM)
Critical files touched: packages/squad-cli/src/cli/shell/index.ts

📦 Modules Affected

root (1 file)

.changeset/audit-onboarding-skill-guards.md

squad-cli (2 files)

packages/squad-cli/src/cli/core/upgrade.ts
packages/squad-cli/src/cli/shell/index.ts

tests (2 files)

test/cli/upgrade.test.ts
test/init-autocast.test.ts

⚠️ Critical Files

packages/squad-cli/src/cli/shell/index.ts

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

github-actions · 2026-04-05T19:35:02Z

🛫 PR Readiness Check

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

PR Scope: 📦🔧 Mixed (product + infrastructure)

⚠️ 4 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	dev is 3 commit(s) ahead — rebase recommended
❌	Copilot review	No Copilot review yet — it may still be processing
✅	Changeset present	Changeset file found
✅	Scope clean	No .squad/ or docs/proposals/ files
✅	No merge conflicts	No merge conflicts
❌	Copilot threads resolved	1 unresolved Copilot thread(s) — fix and resolve before merging
❌	CI passing	14 check(s) still running

Files Changed (5 files, +215 −0)

File	+/−
`.changeset/audit-onboarding-skill-guards.md`	+5 −0
`packages/squad-cli/src/cli/core/upgrade.ts`	+18 −0
`packages/squad-cli/src/cli/shell/index.ts`	+20 −0
`test/cli/upgrade.test.ts`	+99 −0
`test/init-autocast.test.ts`	+73 −0

Total: +215 −0

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

github-actions · 2026-04-05T19:35:03Z

🏗️ Architectural Review

⚠️ Architectural review: 1 warning(s).

Severity	Category	Finding	Files
🟡 warning	bootstrap-area	1 file(s) in the bootstrap area (packages/squad-cli/src/cli/core/) were modified. These files must maintain zero external dependencies. Review carefully.	`packages/squad-cli/src/cli/core/upgrade.ts`

Automated architectural review — informational only.

Copilot

Pull request overview

This PR adds two defensive safeguards to the Squad CLI: (1) a production guard in the interactive shell onboarding flow to prevent dispatching to the coordinator when the roster wasn’t actually populated, and (2) a warning during squad upgrade when customized built-in skills are about to be overwritten.

Changes:

Add a post-createTeam() roster validation in finalizeCast() that aborts dispatch, cleans up .init-prompt, and resets UI state when the roster is empty.
Warn during both upgrade code paths when an on-disk skill differs from the built-in template (with CRLF normalization to reduce false positives).
Add a changeset for the CLI patch release.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
packages/squad-cli/src/cli/shell/index.ts	Adds a guard to prevent coordinator dispatch when team creation yields an empty roster and cleans up `.init-prompt` to avoid retry loops.
packages/squad-cli/src/cli/core/upgrade.ts	Adds skill-overwrite customization warnings in both manifest upgrade and “already current” ensure-sync paths.
.changeset/audit-onboarding-skill-guards.md	Declares a patch changeset describing the two additive audit fixes.

Copilot · 2026-04-06T01:28:34Z

+/**
+ * Warn when a built-in skill has been customized and is about to be overwritten.
+ * Normalizes line endings before comparison to avoid false positives from CRLF differences.
+ */
+function warnIfSkillCustomized(srcPath: string, destPath: string, sourceName: string): void {
+  if (!storage.existsSync(destPath)) return;
+  const existing = (storage.readSync(destPath) ?? '').replace(/\r\n/g, '\n');
+  const template = (storage.readSync(srcPath) ?? '').replace(/\r\n/g, '\n');
+  if (existing && existing !== template) {
+    const skillName = sourceName.split('/')[1] ?? sourceName;
+    warn(`Skill '${skillName}' has been customized — overwriting with built-in version`);
+  }
+}


warnIfSkillCustomized() and its new call sites add a behavioral guarantee (warn only when overwriting a customized built-in skill, with CRLF normalization), but existing upgrade tests don’t assert the warning is emitted (or not emitted) in the key cases. Please add tests that (1) modified skill content triggers warn, (2) CRLF-only differences do not, and (3) identical content does not.

@copilot apply changes based on this feedback

Tests already present in commit ea53d68. Four cases covered in test/cli/upgrade.test.ts: (1) modified skill content triggers warning, (2) CRLF-only differences do NOT trigger warning, (3) identical content does NOT trigger warning, (4) full version upgrade path with customized skill triggers warning. All 28 tests pass.

…#867 review feedback) Agent-Logs-Url: https://github.com/bradygaster/squad/sessions/90986e85-287f-47c5-a192-0ada9af52763 Co-authored-by: diberry <41597107+diberry@users.noreply.github.com>

Item 1: After team creation in finalizeCast(), verify roster is actually populated before re-dispatching to coordinator. Cleans up .init-prompt on failure to prevent auto-retry loops. Item 2: During squad upgrade, warn when a customized built-in skill is about to be overwritten. Warning appears in both the manifest upgrade path and the syncAllSkills ensure-check path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

tamirdresher

Approved by @tamirdresher — reviewed diff, CI green, changes are clean and well-tested. cc @diberry

Item 1: After team creation in finalizeCast(), verify roster is actually populated before re-dispatching to coordinator. Cleans up .init-prompt on failure to prevent auto-retry loops. Item 2: During squad upgrade, warn when a customized built-in skill is about to be overwritten. Warning appears in both the manifest upgrade path and the syncAllSkills ensure-check path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

bradygaster · 2026-04-25T22:43:28Z

Closed by #theSquadsquad triage — verified fixed in v0.9.4.

diberry marked this pull request as ready for review April 6, 2026 01:24

Copilot AI review requested due to automatic review settings April 6, 2026 01:24

Copilot started reviewing on behalf of diberry April 6, 2026 01:25

View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

Copilot started work on behalf of diberry April 6, 2026 04:09

View session

Copilot finished work on behalf of diberry April 6, 2026 04:15

diberry force-pushed the squad/audit-onboarding-skill-guards branch 2 times, most recently from 8cd5ab9 to ea53d68 Compare April 6, 2026 14:57

diberry force-pushed the squad/audit-onboarding-skill-guards branch from ea53d68 to 9380ec9 Compare April 6, 2026 16:08

tamirdresher approved these changes Apr 7, 2026

View reviewed changes

Merge branch 'dev' into squad/audit-onboarding-skill-guards

8f6aa34

diberry merged commit 28697c4 into dev Apr 7, 2026
11 checks passed

bradygaster deleted the squad/audit-onboarding-skill-guards branch August 8, 2026 08:42

Navigation Menu

Repository navigation

fix: add onboarding roster guard and skill overwrite warning - #867

fix: add onboarding roster guard and skill overwrite warning#867
diberry merged 2 commits into
devfrom
squad/audit-onboarding-skill-guards

diberry commented Apr 5, 2026 •

edited by Copilot AI

Loading

Uh oh!

github-actions Bot commented Apr 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

diberry Apr 6, 2026

Uh oh!

diberry Apr 6, 2026

Uh oh!

tamirdresher left a comment

Uh oh!

Uh oh!

bradygaster commented Apr 25, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

diberry commented Apr 5, 2026 • edited by Copilot AI Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Architecture Audit — 2 Medium-Priority Fixes

Item 1: Onboarding flow production guard

Item 2: Skill upgrade overwrite warning

Item 3: Regression tests for finalizeCast roster guard

Notes

Uh oh!

github-actions Bot commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟡 Impact Analysis — PR #867

📊 Summary

🎯 Risk Factors

📦 Modules Affected

⚠️ Critical Files

Uh oh!

github-actions Bot commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛫 PR Readiness Check

⚠️ 4 item(s) to address before review

Files Changed (5 files, +215 −0)

Uh oh!

github-actions Bot commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🏗️ Architectural Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

diberry Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

diberry Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

tamirdresher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bradygaster commented Apr 25, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

diberry commented Apr 5, 2026 •

edited by Copilot AI

Loading

github-actions Bot commented Apr 5, 2026 •

edited

Loading

github-actions Bot commented Apr 5, 2026 •

edited

Loading

github-actions Bot commented Apr 5, 2026 •

edited

Loading