Skip to content

[squad] Implement #53519: Add docs integration, test coverage, and regression checks #53750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 1 commit into from
Aug 18, 2026
+276 −92
Merged

[squad] Implement #53519: Add docs integration, test coverage, and regression checks #53750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1198,6 +1198,12 @@ test-docs-remark:
@node docs/src/lib/remark/inlineMarkdownInHtml.test.js
@echo "✓ Docs remark plugin unit tests passed"

.PHONY: test-docs-wizard-model
test-docs-wizard-model:
@echo "Running AW wizard data model unit tests..."
@node docs/src/lib/wizard/model.test.js
@echo "✓ AW wizard data model unit tests passed"

# Sync templates from .github to pkg/cli/templates
# Sync action pins from .github/aw to pkg/actionpins/data and pkg/workflow/data
.PHONY: sync-action-pins
Expand Down
1 change: 1 addition & 0 deletions docs/astro.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ export default defineConfig({
label: 'Setup',
items: [
{ label: 'Quick Start', link: '/setup/quick-start/' },
{ label: 'AW Wizard', link: '/wizard/' },
{ label: 'Creating Workflows', link: '/setup/creating-workflows/' },
{ label: 'CLI Commands', link: '/setup/cli/' },
],
Expand Down
140 changes: 140 additions & 0 deletions docs/src/lib/wizard/model.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
#!/usr/bin/env node
// @ts-check

/**
* Unit tests for the AW wizard shared JSON data model validation
* (docs/src/lib/wizard/validation.ts).
*
* These tests exercise `validateModel` directly so malformed wizard data
* model JSON is caught automatically instead of only failing at Astro
* build/import time.
*
* Run with: node docs/src/lib/wizard/model.test.js
*/

import { validateModel } from './validation.ts';

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/tdd] Importing validation.ts with a .ts extension relies on Node 22's --experimental-strip-types, which is absent from the existing test infrastructure and undocumented in the Makefile target.

💡 Suggestion

The existing inlineMarkdownInHtml.test.js imports only .js files. The new test imports a .ts file directly, which works on Node 22+ (strip-types is on by default) but will silently fail on older Node versions with a cryptic module-not-found error.

Add a minimum Node version guard in the Makefile target:

test-docs-wizard-model:
	`@node` --version | grep -qE '^v(2[2-9]|[3-9][0-9])' || (echo "Node 22+ required for TypeScript strip-types"; exit 1)
	`@node` docs/src/lib/wizard/model.test.js

Alternatively, document the Node >=22 requirement in the target comment so future CI upgrades know why.

@copilot please address this.

import validModel from '../../data/wizard-data-model.json' with { type: 'json' };

let passed = 0;
let failed = 0;

function assertThrows(fn, label) {
try {
fn();
console.error(` ✗ ${label}`);
console.error(' expected: function to throw');
console.error(' actual: no error was thrown');
failed++;
} catch (error) {
console.log(` ✓ ${label} (${/** @type {Error} */ (error).message})`);
passed++;
}
}

function assertDoesNotThrow(fn, label) {
try {
fn();
console.log(` ✓ ${label}`);
passed++;
} catch (error) {
console.error(` ✗ ${label}`);
console.error(` expected: function not to throw`);
console.error(` actual: threw ${/** @type {Error} */ (error).message}`);
failed++;
}
}

function clone(data) {
return JSON.parse(JSON.stringify(data));
}

// -------------------------------------------------------------------
// Test: the real, checked-in wizard data model is valid
// -------------------------------------------------------------------
console.log('\nreal wizard-data-model.json:');
{
assertDoesNotThrow(() => validateModel(clone(validModel)), 'checked-in wizard-data-model.json passes validation');
}

// -------------------------------------------------------------------
// Test: non-object input is rejected
// -------------------------------------------------------------------
console.log('\nnon-object input:');
{
assertThrows(() => validateModel(null), 'null is rejected');
assertThrows(() => validateModel('not an object'), 'string is rejected');
assertThrows(() => validateModel([]), 'array is rejected');
}

// -------------------------------------------------------------------
// Test: missing/invalid version field
// -------------------------------------------------------------------
console.log('\nversion field:');
{
const missingVersion = clone(validModel);
delete missingVersion.version;
assertThrows(() => validateModel(missingVersion), 'missing version is rejected');

const nonSemverVersion = clone(validModel);
nonSemverVersion.version = '1.0';
Comment on lines +55 to +79

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

L55-79: stdlib: hand-rolled assertThrows/assertDoesNotThrow reimplement assert.throws/assert.doesNotThrow from Node's built-in assert module. Use require('node:assert') (strict variant) instead.

assertThrows(() => validateModel(nonSemverVersion), 'non-semver version ("1.0") is rejected');
}

// -------------------------------------------------------------------
// Test: empty top-level arrays are rejected
// -------------------------------------------------------------------
console.log('\nempty top-level arrays:');
{
const emptyGoals = clone(validModel);
emptyGoals.goalCategories = [];
assertThrows(() => validateModel(emptyGoals), 'empty goalCategories is rejected');

const emptyTriggers = clone(validModel);
emptyTriggers.triggerOptions = [];
assertThrows(() => validateModel(emptyTriggers), 'empty triggerOptions is rejected');

const emptyDestinations = clone(validModel);
emptyDestinations.destinationOptions = [];
assertThrows(() => validateModel(emptyDestinations), 'empty destinationOptions is rejected');
}

// -------------------------------------------------------------------

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/tdd] Test label says 'goal category missing text.label is rejected' but the mutation deletes the entire text object — a broader failure than the label implies.

💡 Suggestion

Rename the label to match the actual mutation, or tighten the test to delete only text.label to precisely exercise the hasText helper's typeof value.label === 'string' branch:

const missingGoalTextLabel = clone(validModel);
delete missingGoalTextLabel.goalCategories[0].text.label;
assertThrows(() => validateModel(missingGoalTextLabel), 'goal category missing text.label is rejected');

@copilot please address this.

// Test: goal category missing required fields
// -------------------------------------------------------------------
console.log('\ngoal category shape:');
{
const missingTriggerIds = clone(validModel);
delete missingTriggerIds.goalCategories[0].triggerOptionIds;
assertThrows(() => validateModel(missingTriggerIds), 'goal category missing triggerOptionIds is rejected');

const missingDestinationIds = clone(validModel);
delete missingDestinationIds.goalCategories[0].destinationOptionIds;
assertThrows(() => validateModel(missingDestinationIds), 'goal category missing destinationOptionIds is rejected');

const missingGoalText = clone(validModel);
delete missingGoalText.goalCategories[0].text;
assertThrows(() => validateModel(missingGoalText), 'goal category missing text.label is rejected');
}

// -------------------------------------------------------------------
// Test: trigger/destination option missing required fields
// -------------------------------------------------------------------
console.log('\ntrigger/destination option shape:');
{
const missingTriggerType = clone(validModel);
delete missingTriggerType.triggerOptions[0].type;
assertThrows(() => validateModel(missingTriggerType), 'trigger option missing type is rejected');

const missingDestinationSafeOutputType = clone(validModel);
delete missingDestinationSafeOutputType.destinationOptions[0].safeOutputType;
assertThrows(
() => validateModel(missingDestinationSafeOutputType),
'destination option missing safeOutputType is rejected',
);
}

// -------------------------------------------------------------------
// Summary
// -------------------------------------------------------------------
console.log(`\n${passed} passed, ${failed} failed`);
if (failed > 0) process.exit(1);
104 changes: 12 additions & 92 deletions docs/src/lib/wizard/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,96 +1,16 @@
import modelData from '../../data/wizard-data-model.json';

export interface WizardText {
label: string;
help?: string;
}

export interface WizardGoalCategory {
id: string;
text: WizardText;
triggerOptionIds: string[];
destinationOptionIds: string[];
defaultTriggerOptionId?: string;
defaultDestinationOptionId?: string;
}

export interface WizardTriggerOption {
id: string;
type: string;
text: WizardText;
frontmatter?: Record<string, unknown>;
}

export interface WizardDestinationOption {
id: string;
safeOutputType: string;
text: WizardText;
inferFromTriggerTypes?: string[];
frontmatter?: Record<string, unknown>;
}

export interface WizardPromptSection {
id: string;
heading: string;
}

export interface WizardDataModel {
version: string;
goalCategories: WizardGoalCategory[];
triggerOptions: WizardTriggerOption[];
destinationOptions: WizardDestinationOption[];
promptTemplate?: {
introText?: string;
sections?: WizardPromptSection[];
};
}

function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value);
}

function hasText(value: unknown): value is WizardText {
return isRecord(value) && typeof value.label === 'string' && (value.help === undefined || typeof value.help === 'string');
}

function validateModel(data: unknown): asserts data is WizardDataModel {
if (!isRecord(data)) throw new Error('Wizard data model must be an object.');
if (typeof data.version !== 'string') throw new Error('Wizard data model version must be a string.');
if (!/^\d+\.\d+\.\d+$/.test(data.version)) throw new Error('Wizard data model version must use semver.');
if (!Array.isArray(data.goalCategories) || data.goalCategories.length === 0) {
throw new Error('Wizard data model requires at least one goal category.');
}
if (!Array.isArray(data.triggerOptions) || data.triggerOptions.length === 0) {
throw new Error('Wizard data model requires at least one trigger option.');
}
if (!Array.isArray(data.destinationOptions) || data.destinationOptions.length === 0) {
throw new Error('Wizard data model requires at least one destination option.');
}

for (const goal of data.goalCategories) {
if (!isRecord(goal) || typeof goal.id !== 'string' || !hasText(goal.text)) {
throw new Error('Each wizard goal category must include id and text.label.');
}
if (!Array.isArray(goal.triggerOptionIds) || goal.triggerOptionIds.length === 0) {
throw new Error(`Goal category "${goal.id}" must define triggerOptionIds.`);
}
if (!Array.isArray(goal.destinationOptionIds) || goal.destinationOptionIds.length === 0) {
throw new Error(`Goal category "${goal.id}" must define destinationOptionIds.`);
}
}

for (const trigger of data.triggerOptions) {
if (!isRecord(trigger) || typeof trigger.id !== 'string' || typeof trigger.type !== 'string' || !hasText(trigger.text)) {
throw new Error('Each wizard trigger option must include id, type, and text.label.');
}
}

for (const destination of data.destinationOptions) {
if (!isRecord(destination) || typeof destination.id !== 'string' || typeof destination.safeOutputType !== 'string' || !hasText(destination.text)) {
throw new Error('Each wizard destination option must include id, safeOutputType, and text.label.');
}
}
}
import { validateModel, type WizardDataModel } from './validation';

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/codebase-design] type WizardDataModel is imported on line 3 but never used locally — it is re-exported via the separate export type { ... } from './validation' block on lines 4–10.

💡 Suggestion

Remove the redundant named import to keep the module header minimal:

import { validateModel } from './validation';

All types flow to consumers exclusively through the re-export block below.

@copilot please address this.

export type {
WizardText,
WizardGoalCategory,
WizardTriggerOption,
WizardDestinationOption,
WizardPromptSection,
WizardDataModel,
} from './validation';

export { validateModel } from './validation';

validateModel(modelData);

Expand Down
97 changes: 97 additions & 0 deletions docs/src/lib/wizard/validation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
export interface WizardText {
label: string;
help?: string;
}

export interface WizardGoalCategory {
id: string;
text: WizardText;
triggerOptionIds: string[];
destinationOptionIds: string[];
defaultTriggerOptionId?: string;
defaultDestinationOptionId?: string;
}

export interface WizardTriggerOption {
id: string;
type: string;
text: WizardText;
frontmatter?: Record<string, unknown>;
}

export interface WizardDestinationOption {
id: string;
safeOutputType: string;
text: WizardText;
inferFromTriggerTypes?: string[];
frontmatter?: Record<string, unknown>;
}

export interface WizardPromptSection {
id: string;
heading: string;
}

export interface WizardDataModel {
version: string;
goalCategories: WizardGoalCategory[];
triggerOptions: WizardTriggerOption[];
destinationOptions: WizardDestinationOption[];
promptTemplate?: {
introText?: string;
sections?: WizardPromptSection[];
};
}

function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value);
}

function hasText(value: unknown): value is WizardText {
return isRecord(value) && typeof value.label === 'string' && (value.help === undefined || typeof value.help === 'string');
}

/**
* Validates that `data` conforms to the shared wizard JSON data model shape,
* throwing a descriptive Error on the first violation found. Used both at
* module load time (via model.ts) and by automated tests to catch malformed
* model data before it reaches the docs build.
*/
export function validateModel(data: unknown): asserts data is WizardDataModel {
if (!isRecord(data)) throw new Error('Wizard data model must be an object.');
if (typeof data.version !== 'string') throw new Error('Wizard data model version must be a string.');
if (!/^\d+\.\d+\.\d+$/.test(data.version)) throw new Error('Wizard data model version must use semver.');
if (!Array.isArray(data.goalCategories) || data.goalCategories.length === 0) {
throw new Error('Wizard data model requires at least one goal category.');
}
if (!Array.isArray(data.triggerOptions) || data.triggerOptions.length === 0) {
throw new Error('Wizard data model requires at least one trigger option.');
}
if (!Array.isArray(data.destinationOptions) || data.destinationOptions.length === 0) {
throw new Error('Wizard data model requires at least one destination option.');
}

for (const goal of data.goalCategories) {
if (!isRecord(goal) || typeof goal.id !== 'string' || !hasText(goal.text)) {
throw new Error('Each wizard goal category must include id and text.label.');
}
if (!Array.isArray(goal.triggerOptionIds) || goal.triggerOptionIds.length === 0) {
throw new Error(`Goal category "${goal.id}" must define triggerOptionIds.`);
}
if (!Array.isArray(goal.destinationOptionIds) || goal.destinationOptionIds.length === 0) {
throw new Error(`Goal category "${goal.id}" must define destinationOptionIds.`);
Comment on lines +78 to +82
}
}

for (const trigger of data.triggerOptions) {
if (!isRecord(trigger) || typeof trigger.id !== 'string' || typeof trigger.type !== 'string' || !hasText(trigger.text)) {
throw new Error('Each wizard trigger option must include id, type, and text.label.');
}
}

for (const destination of data.destinationOptions) {
if (!isRecord(destination) || typeof destination.id !== 'string' || typeof destination.safeOutputType !== 'string' || !hasText(destination.text)) {
throw new Error('Each wizard destination option must include id, safeOutputType, and text.label.');
}
}
}
20 changes: 20 additions & 0 deletions docs/tests/wizard.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,24 @@ test.describe('AW wizard page', () => {
await expect(page.locator('[data-wizard-preview]')).toContainText('Destination: Post a discussion');
await expect(page.getByRole('button', { name: 'Review selections' })).toBeVisible();
});

test('infers a comment destination for the issue automation goal', async ({ page }) => {
await page.goto('/gh-aw/wizard/');
await page.waitForLoadState('networkidle');

await expect(page.getByText('Step 1 of 3')).toBeVisible();
await page.getByLabel('Automate issue triage or replies').check();
await page.getByRole('button', { name: 'Next' }).click();

await expect(page.getByRole('heading', { name: 'Choose a trigger' })).toBeVisible();
await expect(page.getByLabel('When an issue is opened')).toBeVisible();
await expect(page.getByLabel('When someone comments on an issue')).toBeVisible();

await page.getByRole('button', { name: 'Next' }).click();

await expect(page.getByRole('heading', { name: 'Review the inferred output destination' })).toBeVisible();
await expect(page.locator('[data-wizard-summary]')).toContainText('Automate issue triage or replies');
await expect(page.locator('[data-wizard-preview]')).toContainText('Destination: Post a comment');
await expect(page.getByRole('button', { name: 'Review selections' })).toBeVisible();

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/tdd] The test advances past step 2 without asserting which trigger option is selected, making the destination-inference assertion fragile.

💡 Suggestion

The destination "Post a comment" is inferred from the selected trigger type. If the wizard's default trigger changes, this test will silently start testing the wrong path. Explicitly select a trigger before clicking Next so the test reads as a specification:

await page.getByLabel('When an issue is opened').check();
await page.getByRole('button', { name: 'Next' }).click();

This makes the intent clear and decouples the assertion from whatever the wizard happens to pre-select.

@copilot please address this.

});
});
Loading