Repository navigation
feat: add squad health readiness checks - #1877
Conversation
🟠 Impact Analysis — PR #1877Risk tier: 🟠 HIGH 📊 Summary
🎯 Risk Factors
📦 Modules Affecteddocs (1 file)
root (3 files)
squad-cli (4 files)
squad-sdk (3 files)
templates (1 file)
tests (5 files)
This report is generated automatically for every PR. See #733 for details. |
🛫 PR Readiness Check
PR Scope: 📦🔧 Mixed (product + infrastructure)
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 3 commits — consider squashing before review |
| ✅ | 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 | 0 active Copilot thread(s) resolved (6 outdated skipped) |
| ❌ | CI passing | 1 check(s) failing: test |
Files Changed (17 files, +1983 −73)
| File | +/− |
|---|---|
.changeset/1605-squad-health.md |
+6 −0 |
.squad-templates/routing.md |
+3 −8 |
docs/src/content/docs/guide/gh-aw.md |
+1 −1 |
packages/squad-cli/src/cli-entry.ts |
+9 −2 |
packages/squad-cli/src/cli/commands/health.ts |
+797 −0 |
packages/squad-cli/src/cli/core/effective-squad-dir.ts |
+18 −2 |
packages/squad-cli/templates/routing.md |
+3 −8 |
packages/squad-sdk/src/agents/charter-compiler.ts |
+11 −0 |
packages/squad-sdk/src/config/markdown-migration.ts |
+112 −30 |
packages/squad-sdk/templates/routing.md |
+3 −8 |
templates/routing.md |
+3 −8 |
test/agents.test.ts |
+18 −1 |
test/cli/health-command.test.ts |
+813 −0 |
test/effective-squad-dir.test.ts |
+15 −0 |
test/gh-aw-quality.test.ts |
+108 −0 |
test/migration.test.ts |
+47 −0 |
workflows/shared/squad.md |
+16 −5 |
Total: +1983 −73
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
There was a problem hiding this comment.
🟡 Changes recommended
Critical unresolved workflow pin, publication, and health-reporting issues block approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a deterministic squad health readiness command and integrates fail-fast validation into the shared gh-aw bootstrap.
Changes:
- Adds JSON and human-readable health reports.
- Extends routing, team, and charter parsing.
- Updates routing templates, tests, and release metadata.
- Gates dispatch and artifact upload on health checks.
File summaries
| File | Summary |
|---|---|
workflows/shared/squad.md |
Adds the health gate and CLI pin. Critical (3 votes): pin header and guide version consistency tests fail. Critical (1 vote): 0.14.0 is not yet published. |
test/migration.test.ts |
Tests parser enhancements. |
test/gh-aw-quality.test.ts |
Tests workflow ordering and failure behavior. |
test/cli/health-command.test.ts |
Tests health behavior and output contracts. |
test/agents.test.ts |
Tests legacy charter parsing. |
templates/routing.md |
Updates routing template. |
packages/squad-sdk/templates/routing.md |
Updates SDK routing template. |
packages/squad-sdk/src/config/markdown-migration.ts |
Improves team and routing parsing. |
packages/squad-sdk/src/agents/charter-compiler.ts |
Supports legacy charter headings. |
packages/squad-cli/templates/routing.md |
Updates CLI routing template. |
packages/squad-cli/src/cli/commands/health.ts |
Moderate (2 votes): rejects valid @copilot routes. Critical (1 vote): ignores remote config.teamRoot. Critical (1 vote): invalid external paths can prevent structured health output. |
packages/squad-cli/src/cli-entry.ts |
Registers the health command. |
.squad-templates/routing.md |
Removes unresolved routing placeholders. |
.changeset/1605-squad-health.md |
Critical (1 vote): patch release metadata produces 0.13.1, not the pinned 0.14.0. |
Review details
Suppressed comments (7)
packages/squad-cli/src/cli-entry.ts:993
- The command writes the JSON with
console.logimmediately before callingprocess.exit. In CI/npx stdout is commonly a pipe, and forced termination can occur before the stream flushes, truncating the structured report. Setprocess.exitCodeand return instead so Node can flush stdout before exiting.
process.exit(exitCode);
packages/squad-cli/src/cli/commands/health.ts:491
ENV_REFERENCEonly recognizes${VAR}and${env:VAR}. The documented CLI MCP configuration uses the bare"$GITHUB_TOKEN"form (docs/src/content/docs/features/mcp.md:181), so an unset token is never added tovariablesand this check can incorrectly returnskip/pass. Support bare$VARreferences with appropriate boundaries and add a regression test.
const ENV_REFERENCE = /\$\{(?:env:)?([A-Z_][A-Z0-9_]*)\}/g;
packages/squad-cli/src/cli/commands/health.ts:490
- The scanner omits
.vscode/settings.json, even though the repository documents that as a supported VS Code MCP configuration and stores servers undercopilot.mcp.servers(docs/src/content/docs/features/mcp.md:190-203). A project can therefore declare${env:GITHUB_TOKEN}there and health will report that no required variables are declared. Include this path and key, with coverage for the supported format.
const ENV_CONFIG_PATHS = [
'.mcp.json',
path.join('.copilot', 'mcp-config.json'),
path.join('.vscode', 'mcp.json'),
] as const;
packages/squad-cli/src/cli/commands/health.ts:490
squad init --mcp-frontmatteris a supported mode and writesmcp-serversenvironment declarations into.github/agents/squad.agent.md, but this list only scans JSON configuration files. A frontmatter-configured project therefore reportsenv-vars: skipeven when it references unset${...}values, allowing the health gate to pass with missing credentials. Include the supported agent-frontmatter declaration in this scan (or reuse the existing frontmatter parser).
const ENV_CONFIG_PATHS = [
'.mcp.json',
path.join('.copilot', 'mcp-config.json'),
path.join('.vscode', 'mcp.json'),
] as const;
packages/squad-cli/src/cli/commands/health.ts:491
- The matcher only accepts uppercase environment names, so valid declarations such as
${db_password}or${env:AzureToken}are silently ignored and theenv-varscheck can returnskip/passwhile those variables are unset. Environment variable names are case-sensitive and are not required to be uppercase; accept the full identifier range here.
const ENV_REFERENCE = /\$\{(?:env:)?([A-Z_][A-Z0-9_]*)\}/g;
templates/routing.md:11
scripts/sync-templates.mjsdefines.squad-templates/as the canonical source and copies it into this mirror, but the canonicalrouting.mdstill contains{Name}placeholder rows. A later sync will overwrite this validated empty table with unresolved placeholder references, so the canonical source and all mirrors need to be updated together.
Preset installation adds concrete routes for the configured team. Add or edit rows
here only when their agent names also exist in the casting registry.
workflows/shared/squad.md:86
- The fallback now installs
0.14.0, butdocs/src/content/docs/guide/gh-aw.mdstill documents0.12.0forSQUAD_CLI_VERSION.test/squad-cli-pin.test.tsexplicitly compares the workflow pin with that table, so this change leaves the suite failing and gives users an incorrect activation version. Update the guide in the same change.
SQUAD_CLI_VERSION: ${{ vars.SQUAD_CLI_VERSION || '0.14.0' }}
- Files reviewed: 14/14 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Closes #1605 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 93d96d1b-dddb-459e-bb47-80aa21616808
🏗️ Architectural Review
Automated architectural review — informational only. |
|
@copilot review |
…alth Co-authored-by: bradygaster <41929050+bradygaster@users.noreply.github.com>
Reviewed the branch and fixed two defects I found in my own changes (b39a45e):
Both have regression tests verified red against the pre-fix state; One CI failure on this PR is pre-existing on |
There was a problem hiding this comment.
🟡 Changes recommended
Critical compatibility gaps can reject valid existing state, and squad health --help lacks the advertised detailed help.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (8)
Previously missed (1) — in code that hasn't changed since the last review.
packages/squad-cli/src/cli-entry.ts:237
cli-entry.tsinterceptssquad health --helpand looks up a dedicated printer, butCOMMAND_HELPhas nohealthentry, so users getNo detailed help is availableinstead of the advertised--jsonusage. The command-help registry explicitly requires a block for new subcommands; add one for this command.
console.log(` ${BOLD}health${RESET} Validate team state for CI (team, registry, routing, backend, env)`);
console.log(` Flags: --json (structured output for CI)`);
packages/squad-cli/src/cli-entry.ts:994
- The new command is not included in the packaging smoke test's
COMMANDSlist, so the installed CLI entrypoint and package routing forhealthare not exercised; the new tests callrunHealthCommanddirectly and bypass this integration. Addhealthto that existing command list so a packaging/import regression is caught.
if (cmd === 'health') {
const { runHealthCommand } = await import('./cli/commands/health.js');
const exitCode = await runHealthCommand(getSquadStartDir(), args.slice(1));
process.exitCode = exitCode;
return;
packages/squad-cli/src/cli/commands/health.ts:191
- The report is advertised as deterministic, but
localeComparewithout an explicit locale or collation uses the runtime's default collation. This also affects the diagnostics and environment-name sorts below, so the same input can produce different JSON array order on runners with different ICU locales. Use a locale-independent comparator (for example, a shared code-unit comparator) for every serialized sort.
const agentIds = Object.keys(registry.agents).sort((a, b) =>
a.localeCompare(b),
);
packages/squad-cli/src/cli/commands/health.ts:784
repoRootis derived from the project-local.squad, but this value is used for both Git-backed state and environment declarations. In linked-team mode with anorphan/two-layerbackend, the SDK therefore runs Git commands against the project repository instead of the linked team repository; in monorepo--mcp-frontmattermode it also misses.github/agents/squad.agent.mdat the parent Git root. Resolve the backend Git root and environment/agent root independently, and add regressions for both layouts.
args: string[],
): Promise<number> {
let report: HealthReport;
try {
const dirs = effectiveSquadDir(cwd);
packages/squad-cli/src/cli/commands/health.ts:290
- The registry schema separates lookup IDs from display identities (
templates/casting-reference.mdusesagent-role-idwithpersistent_name: "CharacterName"), and routing tables contain the latter. This set only records registry IDs, so a valid route to a differingpersistent_nameis reported as an unknown agent and makes health fail. Add each validatedpersistent_nameto the normalized lookup set as well.
for (const [agentId, entry] of Object.entries(registry.agents)) {
if (!isRecord(entry)) {
throw new Error(`registry entry ${agentId} must be an object`);
}
keys.add(normalizeAgentRef(agentId));
}
packages/squad-cli/src/cli/core/effective-squad-dir.ts:56
resolveSquadPaths()supports remote links whose source directory is legacy.ai-team, butpaths.nameis the name of the local marker directory.runLinkalways creates a local.squad/marker even when the target contains.ai-team, so this resolves<teamRoot>/.squadand makes health report the linked legacy team as missing. Resolve the remote directory from the target's actual.squad/.ai-teamlayout and add a legacy remote regression test.
const teamSquadDir =
paths?.mode === 'remote'
? path.join(paths.teamDir, paths.name)
: local.path;
packages/squad-cli/src/cli/core/effective-squad-dir.ts:60
- The field is documented as the directory whose config selects the backend, but for a linked project this points at the remote state directory.
resolveSquadState()readsstateBackendfrompaths.projectDir, andsquad init/backend migration write that field to the local.squad/config.json; consequently health can skip or validate a different backend than runtime. Separate the config root from the backend storage root (the latter must remainstateDir, especially for remote/external local backends) or reuse the runtime state-resolution context.
backendConfigDir: teamSquadDir,
packages/squad-cli/src/cli/core/effective-squad-dir.ts:55
resolveSquadPaths(dest)walks up to the nearest repo state, butdetectSquadDir(dest)only checks the exact directory. Whensquad healthruns from a nested project directory,pathsresolves the root.squadwhilelocal.pathremains<nested>/.squad; this local branch therefore validates the wrong directory and can fail a healthy checkout. Derivelocalfrompaths.projectDirwhenever resolution succeeds, keepingdetectSquadDironly as the no-path fallback.
const local = detectSquadDir(dest);
const paths = resolveSquadPaths(dest);
const teamSquadDir =
paths?.mode === 'remote'
? path.join(paths.teamDir, paths.name)
- Files reviewed: 17/17 changed files
- Comments generated: 2
- Review effort level: Lite
| const normalizedId = normalizeAgentRef(agentId); | ||
| if ( | ||
| normalizedId !== agentId || | ||
| !/^[a-z0-9][a-z0-9-]*$/.test(agentId) |
| // Detect routing table section (also accepts "## Work Type → Agent" used by this project's routing.md) | ||
| if (/##\s*routing\s*table/i.test(trimmed) || /##\s*work\s*type\s*.{0,5}agent/i.test(trimmed)) { | ||
| inTable = true; |
Summary
squad healthwith deterministicsquad-health/v1JSON and useful human outputValidation
squad health --jsonagainst this repository: passvitest@4.1.11; manifests and lockfile were not changedCloses #1605