Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 132 additions & 0 deletions .github/workflows/squad-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,138 @@ jobs:
- name: Run tests
run: npm test

changelog-gate:
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Check feature flag
id: flag
# Default: gate is ENABLED. When vars.SQUAD_CHANGELOG_CHECK is
# undefined (not set in repo/org variables), the bash comparison
# [ "" = "false" ] evaluates to false, so skip stays "false" and
# the gate runs. Set vars.SQUAD_CHANGELOG_CHECK to "false" to
# explicitly disable.
run: |
if [ "${{ vars.SQUAD_CHANGELOG_CHECK }}" = "false" ]; then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "CHANGELOG gate disabled via vars.SQUAD_CHANGELOG_CHECK"
else
echo "skip=false" >> "$GITHUB_OUTPUT"
fi

- name: Check skip label
if: steps.flag.outputs.skip == 'false'
id: label
run: |
LABELS=$(gh pr view ${{ github.event.pull_request.number }} --json labels --jq '.labels[].name' 2>/dev/null || echo "")
if echo "$LABELS" | grep -q "skip-changelog"; then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "Skipping CHANGELOG gate (skip-changelog label present)"
else
echo "skip=false" >> "$GITHUB_OUTPUT"
fi
env:
GH_TOKEN: ${{ github.token }}

- name: Require CHANGELOG update for SDK/CLI source changes
if: steps.flag.outputs.skip == 'false' && steps.label.outputs.skip != 'true'
Comment on lines +135 to +150
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

This label check relies on gh pr view, but the workflow-level permissions: currently only grants contents: read. With that permission set, GITHUB_TOKEN typically cannot read PR metadata (labels), so gh pr view may fail and the skip label will never be recognized (your || echo "" hides the failure). Prefer using github.event.pull_request.labels.*.name in expressions (no API call needed), or explicitly grant pull-requests: read (workflow- or job-level) so label checks work reliably.

Suggested change
- name: Check skip label
if: steps.flag.outputs.skip == 'false'
id: label
run: |
LABELS=$(gh pr view ${{ github.event.pull_request.number }} --json labels --jq '.labels[].name' 2>/dev/null || echo "")
if echo "$LABELS" | grep -q "skip-changelog"; then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "Skipping CHANGELOG gate (skip-changelog label present)"
else
echo "skip=false" >> "$GITHUB_OUTPUT"
fi
env:
GH_TOKEN: ${{ github.token }}
- name: Require CHANGELOG update for SDK/CLI source changes
if: steps.flag.outputs.skip == 'false' && steps.label.outputs.skip != 'true'
- name: Require CHANGELOG update for SDK/CLI source changes
if: steps.flag.outputs.skip == 'false' && !contains(github.event.pull_request.labels.*.name, 'skip-changelog')

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in upstream PR bradygaster#673 -- now uses github.event.pull_request.labels instead of gh pr view.

run: |
BASE="${{ github.event.pull_request.base.sha }}"
HEAD="${{ github.event.pull_request.head.sha }}"
# Three-dot diff (base...head) finds the merge-base automatically,
# so it works correctly even when the PR branch contains merge
# commits from syncing with the base branch. It compares against
# the common ancestor, not the literal base SHA.
CHANGED=$(git diff --name-only "$BASE"..."$HEAD")

SDK_CLI_CHANGED=$(echo "$CHANGED" | grep -E '^packages/squad-(sdk|cli)/src/' || true)
if [ -z "$SDK_CLI_CHANGED" ]; then
echo "No SDK/CLI source changes detected -- CHANGELOG gate not applicable"
exit 0
fi

echo "SDK/CLI source files changed:"
echo "$SDK_CLI_CHANGED"

CHANGELOG_CHANGED=$(echo "$CHANGED" | grep -E '^CHANGELOG\.md$' || true)
if [ -z "$CHANGELOG_CHANGED" ]; then
echo ""
echo "::error::CHANGELOG.md was not updated but SDK/CLI source files were changed."
echo "::error::Please add a CHANGELOG.md entry describing your changes."
echo "::error::If this is intentional, add the 'skip-changelog' label to your PR."
exit 1
fi

echo "CHANGELOG.md updated -- gate passed"

exports-map-check:
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0

- uses: actions/setup-node@v4
with:
node-version: 22

- name: Check feature flag
id: flag
# Default: gate is ENABLED. When vars.SQUAD_EXPORTS_CHECK is
# undefined (not set in repo/org variables), the bash comparison
# [ "" = "false" ] evaluates to false, so skip stays "false" and
# the gate runs. Set vars.SQUAD_EXPORTS_CHECK to "false" to
# explicitly disable.
run: |
if [ "${{ vars.SQUAD_EXPORTS_CHECK }}" = "false" ]; then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "Exports map check disabled via vars.SQUAD_EXPORTS_CHECK"
else
echo "skip=false" >> "$GITHUB_OUTPUT"
fi

- name: Check skip label
if: steps.flag.outputs.skip == 'false'
id: label
run: |
LABELS=$(gh pr view ${{ github.event.pull_request.number }} --json labels --jq '.labels[].name' 2>/dev/null || echo "")
if echo "$LABELS" | grep -q "skip-exports-check"; then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "Skipping exports map check (skip-exports-check label present)"
else
echo "skip=false" >> "$GITHUB_OUTPUT"
fi
env:
GH_TOKEN: ${{ github.token }}

Comment on lines +207 to +220
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

Same issue as the CHANGELOG gate: this uses gh pr view for label detection, but the workflow permissions: only includes contents: read, so the skip-exports-check label may never be detected (and failures are silenced by || echo ""). Use the PR event payload labels directly in if: conditions, or add pull-requests: read permissions so this step can reliably fetch labels.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in upstream PR bradygaster#673 -- same label payload fix applied.

- name: Check for SDK source changes
if: steps.flag.outputs.skip == 'false' && steps.label.outputs.skip != 'true'
id: changes
run: |
BASE="${{ github.event.pull_request.base.sha }}"
HEAD="${{ github.event.pull_request.head.sha }}"
# Three-dot diff (base...head) finds the merge-base automatically,
# so it works correctly even when the PR branch contains merge
# commits from syncing with the base branch.
SDK_CHANGED=$(git diff --name-only "$BASE"..."$HEAD" | grep -E '^packages/squad-sdk/src/' || true)
if [ -z "$SDK_CHANGED" ]; then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "No SDK source changes detected -- exports check not applicable"
else
echo "skip=false" >> "$GITHUB_OUTPUT"
echo "SDK source files changed:"
echo "$SDK_CHANGED"
fi

- name: Verify exports map matches barrel files
if: steps.flag.outputs.skip == 'false' && steps.label.outputs.skip != 'true' && steps.changes.outputs.skip != 'true'
run: node scripts/check-exports-map.mjs

publish-policy:
runs-on: ubuntu-latest
steps:
Expand Down
45 changes: 45 additions & 0 deletions scripts/check-exports-map.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#!/usr/bin/env node
// check-exports-map.mjs -- Verify package.json exports match barrel files.
// Exit 0 if all barrels are mapped, exit 1 with details if any are missing.
// Uses only Node.js built-ins (fs, path).

import { readFileSync, readdirSync, existsSync } from 'node:fs';
import { resolve, join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';

const __dirname = dirname(fileURLToPath(import.meta.url));
const SDK_ROOT = resolve(__dirname, '..', 'packages', 'squad-sdk');
const SRC_DIR = join(SDK_ROOT, 'src');
const PKG_PATH = join(SDK_ROOT, 'package.json');

const pkg = JSON.parse(readFileSync(PKG_PATH, 'utf8'));
const exportsMap = pkg.exports || {};

const srcEntries = readdirSync(SRC_DIR, { withFileTypes: true });
const barrelDirs = srcEntries
.filter((entry) => entry.isDirectory())
.filter((entry) => existsSync(join(SRC_DIR, entry.name, 'index.ts')))
.map((entry) => entry.name);

const missing = [];

for (const dir of barrelDirs) {
const exportKey = `./${dir}`;
if (!exportsMap[exportKey]) {
missing.push({ dir, expectedKey: exportKey });
}
}
Comment on lines +18 to +31
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

The script currently enforces that every top-level src/

/index.ts has a package.json export entry. In the current repo state there are existing barrel dirs (e.g., platform/remote/roles/streams/upstream) with index.ts but no exports entry, so this script will always exit 1 until those exports are added. If the intent is to gate only newly introduced barrels, consider scoping the check to dirs added/changed in the PR (or temporarily allowlisting known-unexported barrels) so unrelated SDK PRs don't get blocked by pre-existing gaps.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

By design -- the gate catches all gaps. The skip-exports-check label is the escape hatch for PRs that intentionally skip this check.


if (missing.length === 0) {
console.log(`Exports map check passed: all ${barrelDirs.length} barrel directories have export entries.`);
process.exit(0);
} else {
console.error(`Exports map check FAILED: ${missing.length} barrel(s) missing from package.json exports.`);
console.error(`This is by design -- new barrel directories must have matching export entries.\n`);
for (const { dir, expectedKey } of missing) {
console.error(` MISSING: "${expectedKey}" (has src/${dir}/index.ts but no export entry)`);
}
console.error(`\nTo fix: add export entries to packages/squad-sdk/package.json "exports" for each missing barrel.`);
console.error('To skip: add the "skip-exports-check" label to your PR to bypass this gate.');
process.exit(1);
}
53 changes: 53 additions & 0 deletions test/check-exports-map.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* check-exports-map.mjs — Script execution test (#104)
*
* Validates that the exports map checker script:
* 1. Executes without crashing (exits 0 or 1, not a runtime error)
* 2. Produces structured output on stdout or stderr
*
* This does NOT test that exports are complete — the script itself
* catches real gaps (e.g., platform, remote, roles, streams, upstream).
* Those missing exports are expected; they are tracked separately.
*/

import { describe, it, expect } from 'vitest';
import { execFile } from 'node:child_process';
import { resolve } from 'node:path';

const SCRIPT_PATH = resolve(process.cwd(), 'scripts', 'check-exports-map.mjs');

function runScript(): Promise<{ code: number | null; stdout: string; stderr: string }> {
return new Promise((res) => {
execFile('node', [SCRIPT_PATH], { cwd: process.cwd() }, (error, stdout, stderr) => {
const code = error ? error.code ?? (error as NodeJS.ErrnoException & { status?: number }).status ?? 1 : 0;
res({ code: typeof code === 'number' ? code : 1, stdout, stderr });
Comment on lines +20 to +23
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

runScript() coerces any non-numeric execFile error code (e.g., spawn ENOENT, signal termination) to 1 and always resolves, which masks real crashes and makes the "exits 0 or 1" assertion pass even when the script didn't run. Consider rejecting/throwing when execFile fails for reasons other than a normal exit code 1, and preserve signal/spawn errors so the test actually fails on runtime errors.

Suggested change
return new Promise((res) => {
execFile('node', [SCRIPT_PATH], { cwd: process.cwd() }, (error, stdout, stderr) => {
const code = error ? error.code ?? (error as NodeJS.ErrnoException & { status?: number }).status ?? 1 : 0;
res({ code: typeof code === 'number' ? code : 1, stdout, stderr });
return new Promise((res, rej) => {
execFile('node', [SCRIPT_PATH], { cwd: process.cwd() }, (error, stdout, stderr) => {
if (!error) {
res({ code: 0, stdout, stderr });
return;
}
const err = error as NodeJS.ErrnoException & { status?: number; code?: number | string };
if (typeof err.code === 'number') {
res({ code: err.code, stdout, stderr });
return;
}
if (typeof err.status === 'number') {
res({ code: err.status, stdout, stderr });
return;
}
// Non-numeric code or missing exit code (e.g., spawn ENOENT, signal termination)
// Treat this as a crash so the test fails instead of masking it as exit code 1.
rej(error);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in upstream PR bradygaster#673 -- runScript now rejects on spawn errors instead of masking as exit code 1.

});
});
}

describe('check-exports-map.mjs', () => {
it('executes without crashing (exits 0 or 1)', async () => {
const { code } = await runScript();
// Exit 0 = all barrels mapped, exit 1 = some missing.
// Both are valid outcomes. A crash would be a non-0/1 code or thrown error.
expect([0, 1]).toContain(code);
});

it('produces output describing the check result', async () => {
const { stdout, stderr } = await runScript();
const combined = stdout + stderr;
// The script always prints either "passed" or "FAILED" in its output
expect(combined).toMatch(/Exports map check (passed|FAILED)/);
});

it('reports MISSING entries with expected format when barrels are unmapped', async () => {
const { code, stderr } = await runScript();
if (code === 1) {
// When the check fails, each missing barrel is reported with a MISSING: prefix
expect(stderr).toContain('MISSING:');
// The error message should mention the skip label escape hatch
expect(stderr).toContain('skip-exports-check');
}
// If code === 0, all barrels are mapped and there is nothing to assert here
});
});
Loading