Skip to content

Add documentation code validation system#356

Merged
patniko merged 14 commits intomainfrom
docs-improvements
Feb 4, 2026
Merged

Add documentation code validation system#356
patniko merged 14 commits intomainfrom
docs-improvements

Conversation

@patniko
Copy link
Contributor

@patniko patniko commented Feb 4, 2026

Summary

Adds an automated system to validate that code examples in documentation are syntactically correct and type-check against the actual SDK APIs.

What's included

Validation tooling (scripts/docs-validation/)

  • extract.ts - Extracts code blocks from markdown files
  • validate.ts - Runs language-specific validation:
    • TypeScript: tsc --noEmit type checking
    • Python: py_compile + mypy for syntax and type checking
    • Go: go build compilation checking
    • C#: dotnet build compilation checking

CI Integration

  • New GitHub Actions workflow (.github/workflows/docs-validation.yml)
  • Runs on PRs touching docs/ or SDK source files
  • Parallel validation jobs for each language
  • Also supports workflow_dispatch and merge_group

Developer Experience

  • just validate-docs - Run all doc validation locally
  • just validate-docs-ts, validate-docs-py, etc. for single languages

Documentation fixes

  • Fixed Go docs: client.Start()client.Start(ctx)
  • Added skip markers for intentionally partial code snippets (config examples, type signatures)

Results

All 157 code blocks now pass validation:

  • ✅ TypeScript: 87 passing
  • ✅ Python: 32 passing
  • ✅ Go: 8 passing
  • ✅ C#: 30 passing
  • 31 blocks skipped (intentional partial snippets)

- Add scripts/docs-validation/ tooling to extract and validate code blocks
- Extract code from markdown and validate with language-specific compilers
- TypeScript: tsc --noEmit type checking
- Python: py_compile + mypy for syntax and type checking
- Go: go build for compilation checking
- C#: dotnet build for compilation checking

- Add GitHub Actions workflow (.github/workflows/docs-validation.yml)
- Runs on PRs touching docs/ or SDK source files
- Parallel validation jobs for each language

- Add justfile tasks: validate-docs, validate-docs-ts, etc.

- Fix Go docs: client.Start() -> client.Start(ctx)
- All 157 code blocks now pass validation
Copilot AI review requested due to automatic review settings February 4, 2026 05:50
@patniko patniko requested a review from a team as a code owner February 4, 2026 05:50
try {
// Run tsc
const tscPath = path.join(ROOT_DIR, "nodejs/node_modules/.bin/tsc");
execSync(`${tscPath} --project ${tsconfigPath} 2>&1`, {

Check warning

Code scanning / CodeQL

Shell command built from environment values Medium

This shell command depends on an uncontrolled
absolute path
.

import * as fs from "fs";
import * as path from "path";
import { execSync, spawn } from "child_process";
if (hasStatements) {
// This is a snippet, wrap it
const lines = code.split("\n");
const packageLine = lines.find((l) => l.startsWith("package ")) || "";
// This is a snippet, wrap it
const lines = code.split("\n");
const packageLine = lines.find((l) => l.startsWith("package ")) || "";
const imports = lines.filter(
- Remove separate extract job and artifact upload/download
- Each language job now extracts and validates independently
- Jobs run in parallel without dependencies
- Remove docs/.gitignore (no longer needed)
Copy link
Contributor

Copilot AI left a comment

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 adds an automated documentation validation system to ensure code examples in documentation are syntactically correct and type-check against the actual SDK APIs.

Changes:

  • New validation tooling that extracts code blocks from markdown files and validates them using language-specific tools (TypeScript: tsc, Python: py_compile + mypy, Go: go build, C#: dotnet build)
  • GitHub Actions CI workflow that runs validation in parallel for each language on PRs touching docs or SDK source
  • Developer commands via justfile for local validation (just validate-docs, just validate-docs-ts, etc.)
  • Documentation fixes including corrected Go examples and skip markers for intentionally partial code snippets

Reviewed changes

Copilot reviewed 20 out of 22 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
scripts/docs-validation/validate.ts Runs language-specific type/compile checks on extracted code blocks
scripts/docs-validation/extract.ts Extracts code blocks from markdown files and wraps them for validation
scripts/docs-validation/package.json Dependencies for validation scripts (glob, tsx, typescript)
scripts/docs-validation/package-lock.json Lock file for validation script dependencies
scripts/docs-validation/.gitignore Ignores node_modules for validation scripts
.github/workflows/docs-validation.yml CI workflow that validates docs on PRs with parallel jobs per language
justfile Adds validate-docs commands following existing patterns
docs/.gitignore Ignores generated .validation directory
docs/mcp/overview.md Fixes Go example: adds context.Background(), Start(ctx), correct MCPServerConfig usage
docs/mcp/debugging.md Adds skip marker for partial config example
docs/hooks/user-prompt-submitted.md Adds skip markers for type signatures, updates hook output examples
docs/hooks/session-lifecycle.md Adds skip markers for Go type signatures
docs/hooks/pre-tool-use.md Adds skip markers for Go type signatures
docs/hooks/post-tool-use.md Adds skip markers for Go type signatures
docs/hooks/overview.md Adds missing await client.start() call, fixes broken link
docs/hooks/error-handling.md Updates error handling examples to use correct field names
docs/guides/skills.md New comprehensive guide for custom skills feature
docs/guides/session-persistence.md Fixes Go examples with context.Background(), corrects Python client instantiation
docs/getting-started.md Fixes all Go examples with context usage, adds event subscription methods section
docs/debugging.md Fixes Python client instantiation syntax, adds skip markers for Go examples
docs/auth/index.md Adds skip markers for partial Go examples
docs/auth/byok.md Fixes Go example with context.Background(), adds skip markers for comparison examples
Files not reviewed (1)
  • scripts/docs-validation/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)

scripts/docs-validation/validate.ts:8

  • The spawn import is declared but never used in this file. Only execSync is used for running commands. This is dead code that should be removed to keep the imports clean.
import { execSync, spawn } from "child_process";

scripts/docs-validation/validate.ts:224

  • Similar cross-platform path issue: the path in the go.mod replace directive contains backslashes on Windows which will cause issues. Go module paths should always use forward slashes regardless of the operating system.

Use path.join(...).replace(/\\/g, '/') to ensure forward slashes in the go.mod file.

replace github.com/github/copilot-sdk/go => ${path.join(ROOT_DIR, "go")}


// Compile all files together
try {
execSync(`dotnet build "${path.join(csDir, "DocsValidation.csproj")}" 2>&1`, {
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The command uses double quotes around the file path which could fail on Windows when the path contains backslashes. While path.join is used (which is good), the resulting path is embedded directly in a shell command string. On Windows, PowerShell or cmd.exe may interpret backslashes differently.

Consider using the shell: true option with execSync or normalizing paths for cross-platform compatibility. Alternatively, use execFile or spawn with an array of arguments instead of a string command to avoid shell escaping issues entirely.

Copilot uses AI. Check for mistakes.
const errors: string[] = [];

try {
// Use go vet for syntax and basic checks
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The comment says "Use go vet for syntax and basic checks" but the actual command is go build, not go vet. This comment is misleading and should be updated to accurately describe what the command does (compilation checking).

Suggested change
// Use go vet for syntax and basic checks
// Compile with go build to check for syntax and basic compile-time errors

Copilot uses AI. Check for mistakes.
<NoWarn>CS8019;CS0168;CS0219</NoWarn>
</PropertyGroup>
<ItemGroup>
<ProjectReference Include="${path.join(ROOT_DIR, "dotnet/src/GitHub.Copilot.SDK.csproj")}" />
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The path includes backslashes in the XML which will cause issues on Windows. When generating file paths for XML (csproj) files that will be processed by dotnet, the paths should use forward slashes or be properly escaped for the platform.

Use path.join(...).replace(/\\/g, '/') or similar to ensure the path uses forward slashes, which work cross-platform in dotnet project files.

Copilot uses AI. Check for mistakes.
}

// Check for await outside function
if (trimmed.includes("await ") && !inAsyncFunction) {
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The async detection logic only checks if await appears in lines that start with "await " (includes a space). However, await can appear in the middle of expressions like result = await foo() or return await bar(). The current logic would miss these cases where await appears after other tokens.

Consider using a more robust regex like /\bawait\s+/ to match await followed by whitespace anywhere in the line, not just using includes("await ").

Suggested change
if (trimmed.includes("await ") && !inAsyncFunction) {
if (/\bawait\s+/.test(trimmed) && !inAsyncFunction) {

Copilot uses AI. Check for mistakes.
Comment on lines +78 to +81
const tscPath = path.join(ROOT_DIR, "nodejs/node_modules/.bin/tsc");
execSync(`${tscPath} --project ${tsconfigPath} 2>&1`, {
encoding: "utf-8",
cwd: tsDir,
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The TypeScript compiler path is constructed using a relative path that assumes the tsc binary exists in the nodejs SDK's node_modules. This could fail if the nodejs SDK dependencies are not installed, or in CI environments where the validation script runs in a different context.

Consider using npx tsc or checking if the tsc binary exists before attempting to use it, with a clear error message if not found.

Suggested change
const tscPath = path.join(ROOT_DIR, "nodejs/node_modules/.bin/tsc");
execSync(`${tscPath} --project ${tsconfigPath} 2>&1`, {
encoding: "utf-8",
cwd: tsDir,
const localTscPath = path.join(ROOT_DIR, "nodejs/node_modules/.bin/tsc");
const tscCommand = fs.existsSync(localTscPath)
? `"${localTscPath}"`
: (() => {
console.warn(
"⚠️ Local TypeScript compiler not found at " +
localTscPath +
". Falling back to `npx tsc`. Ensure TypeScript is available in your environment."
);
return "npx tsc";
})();
execSync(`${tscCommand} --project "${tsconfigPath}"`, {
encoding: "utf-8",
cwd: tsDir,
shell: true,

Copilot uses AI. Check for mistakes.
// Go: add main function if missing and has statements outside functions
if (block.language === "go" && !code.includes("func main()")) {
// Check if code has statements that need to be in main
const hasStatements = /^[a-z]/.test(code.trim().split("\n").pop() || "");
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The logic for detecting if code has "statements that need to be in main" only checks the last line of the code using .pop(). This is insufficient because:

  1. The last line might be empty or a comment
  2. There could be statements earlier in the code that need to be wrapped

This heuristic is too simplistic and may miss cases where Go code needs proper wrapping. Consider checking all non-empty, non-comment lines or using a more robust approach to detect loose statements.

Suggested change
const hasStatements = /^[a-z]/.test(code.trim().split("\n").pop() || "");
const hasStatements = code
.split("\n")
.some((line) => {
const trimmed = line.trim();
if (!trimmed) return false;
if (trimmed.startsWith("//")) return false;
if (trimmed.startsWith("package ")) return false;
if (trimmed.startsWith("import ")) return false;
if (trimmed === "import (" || trimmed === ")") return false;
if (/^(type|func|var|const)\b/.test(trimmed)) return false;
return true;
});

Copilot uses AI. Check for mistakes.
Comment on lines +236 to +242
const rest = lines.filter(
(l) =>
!l.startsWith("package ") &&
!l.startsWith("import ") &&
!l.startsWith("import (") &&
!l.startsWith(")") &&
!l.startsWith("\t") // import block lines
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The filter for import block lines uses !l.startsWith("\t") to exclude lines within an import block, but this assumes tab indentation. If the code uses spaces for indentation in the import block, those lines won't be filtered out and will end up in the rest array, potentially causing issues.

Consider using a more robust check like !/^\s+/.test(l) or tracking whether you're inside an import block more explicitly.

Copilot uses AI. Check for mistakes.

try {
// Use go vet for syntax and basic checks
execSync(`go build -o /dev/null "${fullPath}" 2>&1`, {
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The Go build command uses /dev/null as the output path, which is not compatible with Windows. On Windows, the null device is NUL or nul. This will cause the validation to fail on Windows platforms.

Consider using a platform-agnostic approach. Since this is Node.js, you could use os.devNull from the os module, or simply omit the output file altogether since you're only checking compilation errors.

Copilot uses AI. Check for mistakes.
if (hasStatements) {
// This is a snippet, wrap it
const lines = code.split("\n");
const packageLine = lines.find((l) => l.startsWith("package ")) || "";
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

Unused variable packageLine.

Suggested change
const packageLine = lines.find((l) => l.startsWith("package ")) || "";

Copilot uses AI. Check for mistakes.
Comment on lines +233 to +235
const imports = lines.filter(
(l) => l.startsWith("import ") || l.startsWith('import (')
);
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

Unused variable imports.

Suggested change
const imports = lines.filter(
(l) => l.startsWith("import ") || l.startsWith('import (')
);

Copilot uses AI. Check for mistakes.
…ppets

- Add skip markers to Python type signature definitions (hooks)
- Add skip markers to C# partial config snippets (mcp/debugging.md)
- Add skip markers to Python snippets using undefined variables
Shows a summary table in the PR with:
- Pass/fail status per language
- Total code blocks validated
- Details of any failures
The C# documentation validation was failing because:
1. Multiple files had top-level statements (C# only allows one per project)
2. Some snippets were missing the 'using GitHub.Copilot.SDK;' directive

This fix:
- Wraps C# code snippets without class/namespace in unique static classes
- Preserves existing using directives and adds SDK using if missing
- Detects async code (await keyword) and creates async Main methods
- Skips wrapping for code that already has structure (classes, namespaces, delegates)
These code snippets are intentionally partial - they show specific
configurations assuming a 'client' or 'session' variable already exists.
The validation cannot compile them standalone, so they're skipped.

Also fixed mcp/overview.md to use List<string> instead of string[]
to match the actual SDK types.
@patniko patniko merged commit 02f0d72 into main Feb 4, 2026
14 checks passed
@patniko patniko deleted the docs-improvements branch February 4, 2026 06:21
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.

1 participant