diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 6693fef..a6f9d7f 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -6,7 +6,10 @@ "source=${localEnv:HOME}/.claude,target=/home/vscode/.claude,type=bind,consistency=cached" ], "features": { - "ghcr.io/devcontainers/features/github-cli:1": {} + "ghcr.io/devcontainers/features/github-cli:1": {}, + "ghcr.io/devcontainers/features/node:1": { + "version": "lts" + } }, "postCreateCommand": "curl -fsSL https://claude.ai/install.sh | bash" } diff --git a/.github/ISSUE_TEMPLATE/.gitkeep b/.github/ISSUE_TEMPLATE/.gitkeep deleted file mode 100644 index e69de29..0000000 diff --git a/.github/ISSUE_TEMPLATE/review-finding.yml b/.github/ISSUE_TEMPLATE/review-finding.yml new file mode 100644 index 0000000..5a8819f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/review-finding.yml @@ -0,0 +1,46 @@ +name: Review Finding +description: An issue found during automated or manual code review. +title: "" +labels: ["review-finding"] +body: + - type: textarea + id: finding-description + attributes: + label: Finding description + description: Describe the problem found during review. + placeholder: Explain what is wrong and why it matters. + validations: + required: true + + - type: dropdown + id: severity + attributes: + label: Severity + description: How critical is this finding? + options: + - blocking + - should-fix + - suggestion + validations: + required: true + + - type: textarea + id: affected-files + attributes: + label: Affected files + description: List the files and line numbers where the issue was found. + placeholder: | + - path/to/file1.ts:42 + - path/to/file2.ts:15-23 + render: markdown + validations: + required: true + + - type: textarea + id: suggested-fix + attributes: + label: Suggested fix + description: How should this finding be addressed? + placeholder: Describe the recommended approach to fix this issue. + validations: + required: false diff --git a/.github/ISSUE_TEMPLATE/task.yml b/.github/ISSUE_TEMPLATE/task.yml new file mode 100644 index 0000000..83dff12 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/task.yml @@ -0,0 +1,60 @@ +name: Task +description: A structured task created during planning. Scoped to be completable in a single agent session. +title: "" +labels: ["task"] +body: + - type: textarea + id: summary + attributes: + label: Summary + description: What needs to be done and why. + placeholder: Describe the task in 1-3 sentences. + validations: + required: true + + - type: textarea + id: files-to-modify + attributes: + label: Files to modify + description: List all files that should be created or changed for this task. + placeholder: | + - path/to/file1.ts + - path/to/file2.ts + render: markdown + validations: + required: true + + - type: textarea + id: implementation-steps + attributes: + label: Implementation steps + description: Ordered steps to complete the task. Each step should be concrete and verifiable. + placeholder: | + 1. First step + 2. Second step + 3. Third step + render: markdown + validations: + required: true + + - type: textarea + id: acceptance-criteria + attributes: + label: Acceptance criteria + description: Conditions that must be true for this task to be considered complete. + placeholder: | + - [ ] Criterion 1 + - [ ] Criterion 2 + render: markdown + validations: + required: true + + - type: textarea + id: dependencies + attributes: + label: Dependencies + description: Other issues that must be completed before this task can start. Use issue references. + placeholder: | + Blocked by #XX, #YY + validations: + required: false diff --git a/.github/agent-workflow/.gitkeep b/.github/agent-workflow/.gitkeep deleted file mode 100644 index e69de29..0000000 diff --git a/.github/agent-workflow/commitlint.config.js b/.github/agent-workflow/commitlint.config.js new file mode 100644 index 0000000..ffe85d4 --- /dev/null +++ b/.github/agent-workflow/commitlint.config.js @@ -0,0 +1,7 @@ +module.exports = { + extends: ['@commitlint/config-conventional'], + rules: { + // Enforce 72 char limit on subject line + 'header-max-length': [2, 'always', 72], + } +}; diff --git a/.github/agent-workflow/scripts/guardrail-api-surface.js b/.github/agent-workflow/scripts/guardrail-api-surface.js new file mode 100644 index 0000000..ee5df58 --- /dev/null +++ b/.github/agent-workflow/scripts/guardrail-api-surface.js @@ -0,0 +1,172 @@ +const { hasNonStaleApproval } = require('./lib/approval.js'); +const { detectAPIChanges } = require('./lib/api-patterns.js'); + +module.exports = async function({ github, context, core }) { + const owner = context.repo.owner; + const repo = context.repo.repo; + const prNumber = context.payload.pull_request.number; + const headSha = context.payload.pull_request.head.sha; + const checkName = 'guardrail/api-surface'; + const configuredConclusion = process.env.CONCLUSION || 'action_required'; + + // Check for non-stale PR approval override + const reviews = await github.rest.pulls.listReviews({ + owner, + repo, + pull_number: prNumber, + }); + const hasValidApproval = hasNonStaleApproval(reviews.data, headSha); + + if (hasValidApproval) { + await github.rest.checks.create({ + owner, + repo, + head_sha: headSha, + name: checkName, + status: 'completed', + conclusion: 'neutral', + output: { + title: 'API surface check: approved by reviewer', + summary: 'A non-stale PR approval overrides this guardrail.', + }, + }); + return; + } + + // Get PR files and scan for API surface changes + const files = await github.paginate(github.rest.pulls.listFiles, { + owner, + repo, + pull_number: prNumber, + }); + + // OpenAPI / Swagger file patterns + const openApiFilePatterns = [ + /openapi\.(ya?ml|json)$/i, + /swagger\.(ya?ml|json)$/i, + /api-spec\.(ya?ml|json)$/i, + ]; + + const annotations = []; + let totalApiChanges = 0; + + for (const file of files) { + // Skip removed files + if (file.status === 'removed') continue; + + // Check if this is an OpenAPI/Swagger spec file + const isOpenApiFile = openApiFilePatterns.some((p) => p.test(file.filename)); + if (isOpenApiFile) { + totalApiChanges++; + annotations.push({ + path: file.filename, + start_line: 1, + end_line: 1, + annotation_level: 'warning', + message: `OpenAPI/Swagger spec file modified: ${file.filename}. API contract changes require careful review.`, + }); + continue; + } + + // Use API pattern detection from shared library + if (!file.patch) continue; + + const apiChanges = detectAPIChanges(file.patch, file.filename); + if (apiChanges.length > 0) { + totalApiChanges += apiChanges.length; + + // Parse patch for line numbers + const lines = file.patch.split('\n'); + let currentLine = 0; + let changeIndex = 0; + + for (const line of lines) { + // Track line numbers from hunk headers + const hunkMatch = line.match(/^@@\s+-\d+(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s+@@/); + if (hunkMatch) { + currentLine = parseInt(hunkMatch[1], 10); + continue; + } + + // Only look at added lines + if (line.startsWith('+') && !line.startsWith('+++')) { + if (changeIndex < apiChanges.length) { + annotations.push({ + path: file.filename, + start_line: currentLine, + end_line: currentLine, + annotation_level: 'warning', + message: `API surface change: ${apiChanges[changeIndex]} - ${line.substring(1).trim()}`, + }); + changeIndex++; + } + } + + // Advance line counter for added and context lines + if (!line.startsWith('-')) { + currentLine++; + } + } + } + } + + // Report results + if (totalApiChanges === 0) { + await github.rest.checks.create({ + owner, + repo, + head_sha: headSha, + name: checkName, + status: 'completed', + conclusion: 'success', + output: { + title: 'API surface check: no changes detected', + summary: 'No API surface changes found in this PR.', + }, + }); + } else { + // GitHub API limits annotations to 50 per call + const batchSize = 50; + const batches = []; + for (let i = 0; i < annotations.length; i += batchSize) { + batches.push(annotations.slice(i, i + batchSize)); + } + + const summary = [ + `Found ${totalApiChanges} API surface change(s) across the PR.`, + '', + 'API surface changes have outsized downstream impact. Review these changes carefully.', + '', + 'To override: approve the PR to signal these changes are intentional.', + ].join('\n'); + + // Create the check run with the first batch of annotations + const checkRun = await github.rest.checks.create({ + owner, + repo, + head_sha: headSha, + name: checkName, + status: 'completed', + conclusion: configuredConclusion, + output: { + title: `API surface check: ${totalApiChanges} change(s) detected`, + summary, + annotations: batches[0] || [], + }, + }); + + // If there are more annotations, update the check run with additional batches + for (let i = 1; i < batches.length; i++) { + await github.rest.checks.update({ + owner, + repo, + check_run_id: checkRun.data.id, + output: { + title: `API surface check: ${totalApiChanges} change(s) detected`, + summary, + annotations: batches[i], + }, + }); + } + } +}; diff --git a/.github/agent-workflow/scripts/guardrail-dependencies.js b/.github/agent-workflow/scripts/guardrail-dependencies.js new file mode 100644 index 0000000..b121cf6 --- /dev/null +++ b/.github/agent-workflow/scripts/guardrail-dependencies.js @@ -0,0 +1,87 @@ +const { hasNonStaleApproval } = require('./lib/approval.js'); +const { isDependencyFile } = require('./lib/file-patterns.js'); + +module.exports = async function({ github, context, core }) { + const CHECK_NAME = 'guardrail/dependency-changes'; + const configuredConclusion = process.env.CONCLUSION || 'action_required'; + + // Check for non-stale approving PR review (override mechanism) + const { data: reviews } = await github.rest.pulls.listReviews({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number + }); + + const lastCommitSha = context.payload.pull_request.head.sha; + const hasValidApproval = hasNonStaleApproval(reviews, lastCommitSha); + + if (hasValidApproval) { + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: context.sha, + name: CHECK_NAME, + status: 'completed', + conclusion: 'neutral', + output: { + title: 'Dependency changes: approved by reviewer', + summary: 'A non-stale PR approval overrides this guardrail check.' + } + }); + return; + } + + // Get PR changed files + const files = await github.paginate( + github.rest.pulls.listFiles, + { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + per_page: 100 + } + ); + + const changedDependencyFiles = files.filter(f => isDependencyFile(f.filename)); + + // No dependency files changed: success + if (changedDependencyFiles.length === 0) { + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: context.sha, + name: CHECK_NAME, + status: 'completed', + conclusion: 'success', + output: { + title: 'Dependency changes: no dependency files modified', + summary: 'No dependency manifest or lock files were changed in this PR.' + } + }); + return; + } + + // Dependency files changed: requires human review + const fileList = changedDependencyFiles.map(f => '- `' + f.filename + '`').join('\n'); + const annotations = changedDependencyFiles.map(f => ({ + path: f.filename, + start_line: 1, + end_line: 1, + annotation_level: 'warning', + message: 'Dependency file modified. Human review required before merge.' + })); + + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: context.sha, + name: CHECK_NAME, + status: 'completed', + conclusion: configuredConclusion, + output: { + title: `Dependency changes: ${changedDependencyFiles.length} file(s) modified`, + summary: `Dependency files were modified and require human review before merge.\n\n**Changed dependency files:**\n${fileList}\n\n**To resolve:** A maintainer must submit an approving PR review. The approval must be on the current head commit (non-stale).`, + annotations: annotations + } + }); +}; diff --git a/.github/agent-workflow/scripts/guardrail-scope.js b/.github/agent-workflow/scripts/guardrail-scope.js new file mode 100644 index 0000000..6adaa42 --- /dev/null +++ b/.github/agent-workflow/scripts/guardrail-scope.js @@ -0,0 +1,177 @@ +const { hasNonStaleApproval } = require('./lib/approval.js'); +const { parseFixesReferences } = require('./lib/fixes-parser.js'); +const { extractFilePaths, isInScope } = require('./lib/scope-matcher.js'); + +module.exports = async function({ github, context, core }) { + const checkName = 'guardrail/scope'; + const configuredConclusion = process.env.CONCLUSION || 'action_required'; + + // Helper: create check run + async function createCheckRun(conclusion, title, summary, annotations = []) { + const output = { title, summary }; + if (annotations.length > 0) { + output.annotations = annotations.slice(0, 50); + } + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: context.payload.pull_request.head.sha, + name: checkName, + status: 'completed', + conclusion, + output + }); + } + + // Step 1: Check for non-stale PR approval override + const reviews = await github.rest.pulls.listReviews({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number + }); + const headSha = context.payload.pull_request.head.sha; + const hasValidApproval = hasNonStaleApproval(reviews.data, headSha); + + // Step 3: Parse issue reference from PR body + const prBody = context.payload.pull_request.body || ''; + const issueNumbers = parseFixesReferences(prBody); + if (issueNumbers.length === 0) { + await createCheckRun( + 'success', + 'Scope enforcement: no linked issue', + 'No `fixes #N` reference found in PR description. Scope enforcement skipped.' + ); + return; + } + const issueNumber = issueNumbers[0]; + + // Step 4: Get the issue body + all child issue bodies + const issueBodies = []; + try { + const issue = await github.rest.issues.get({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issueNumber + }); + issueBodies.push(issue.data.body || ''); + } catch (e) { + await createCheckRun( + 'success', + 'Scope enforcement: issue not found', + `Could not read issue #${issueNumber}. Scope enforcement skipped.` + ); + return; + } + + // Query sub-issues via GraphQL + try { + const subIssueResult = await github.graphql(` + query($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + issue(number: $number) { + subIssues(first: 50) { + nodes { body } + } + } + } + } + `, { + owner: context.repo.owner, + repo: context.repo.repo, + number: issueNumber + }); + const children = subIssueResult.repository.issue.subIssues.nodes || []; + for (const child of children) { + if (child.body) issueBodies.push(child.body); + } + } catch (e) { + // Sub-issues query failed — continue with parent body only + } + + // Step 5: Extract file paths from all issue bodies + const scopeFiles = []; + for (const body of issueBodies) { + scopeFiles.push(...extractFilePaths(body)); + } + const uniqueScopeFiles = [...new Set(scopeFiles)]; + + if (uniqueScopeFiles.length === 0) { + await createCheckRun( + 'success', + 'Scope enforcement: no files listed in issues', + `Issue #${issueNumber} and its children do not list any file paths. Scope enforcement skipped.` + ); + return; + } + + // Step 6: Get changed files from the PR + const changedFiles = []; + let page = 1; + while (true) { + const resp = await github.rest.pulls.listFiles({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + per_page: 100, + page + }); + changedFiles.push(...resp.data); + if (resp.data.length < 100) break; + page++; + } + + // Step 7: Compare changed files against scope + const outOfScope = changedFiles.filter(f => !isInScope(f.filename, uniqueScopeFiles)); + + // Step 8: Report results + if (outOfScope.length === 0) { + await createCheckRun( + 'success', + 'Scope enforcement: all files in scope', + `All ${changedFiles.length} changed files are within scope of issue #${issueNumber} and its children.` + ); + return; + } + + // There are out-of-scope files + if (hasValidApproval) { + await createCheckRun( + 'neutral', + `Scope enforcement: approved by reviewer (${outOfScope.length} files outside scope)`, + `PR modifies ${outOfScope.length} file(s) not listed in issue #${issueNumber} or its children, but a non-stale approval exists.\n\nOut-of-scope files:\n${outOfScope.map(f => '- `' + f.filename + '`').join('\n')}` + ); + return; + } + + // Build annotations for out-of-scope files + const annotations = outOfScope.map(f => ({ + path: f.filename, + start_line: 1, + end_line: 1, + annotation_level: 'warning', + message: `This file is not listed in the task scope for issue #${issueNumber} or its children. If this change is intentional, approve the PR to override.` + })); + + // Determine conclusion based on config and severity + const isMinor = outOfScope.length <= 2; + const conclusion = isMinor ? 'neutral' : configuredConclusion; + + const summary = [ + `PR modifies ${outOfScope.length} file(s) not listed in issue #${issueNumber} or its children.`, + '', + '**Out-of-scope files:**', + ...outOfScope.map(f => `- \`${f.filename}\``), + '', + `**In-scope files (from issues):**`, + ...uniqueScopeFiles.map(f => `- \`${f}\``), + '', + 'To resolve: either update the issue to include these files, or approve the PR to override this check.' + ].join('\n'); + + await createCheckRun( + conclusion, + `Scope enforcement: ${outOfScope.length} file(s) outside task scope`, + summary, + annotations + ); +}; diff --git a/.github/agent-workflow/scripts/guardrail-test-ratio.js b/.github/agent-workflow/scripts/guardrail-test-ratio.js new file mode 100644 index 0000000..9ff8f9b --- /dev/null +++ b/.github/agent-workflow/scripts/guardrail-test-ratio.js @@ -0,0 +1,137 @@ +const { hasNonStaleApproval } = require('./lib/approval.js'); +const { isTestFile, isCodeFile } = require('./lib/file-patterns.js'); + +module.exports = async function({ github, context, core }) { + const configuredConclusion = process.env.CONCLUSION || 'action_required'; + const threshold = parseFloat(process.env.THRESHOLD) || 0.5; + + // Get PR files + const prNumber = context.payload.pull_request.number; + const allFiles = []; + let page = 1; + while (true) { + const { data: files } = await github.rest.pulls.listFiles({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + per_page: 100, + page: page, + }); + allFiles.push(...files); + if (files.length < 100) break; + page++; + } + + // Categorize files and count lines + let testLines = 0; + let implLines = 0; + const implFilesWithNoTests = []; + + for (const file of allFiles) { + if (file.status === 'removed') continue; + if (!isCodeFile(file.filename) && !isTestFile(file.filename)) continue; + + const additions = file.additions || 0; + + if (isTestFile(file.filename)) { + testLines += additions; + } else { + implLines += additions; + implFilesWithNoTests.push({ + filename: file.filename, + additions: additions, + }); + } + } + + core.info(`Test lines added: ${testLines}`); + core.info(`Implementation lines added: ${implLines}`); + + // Handle edge case: no implementation lines + if (implLines === 0) { + core.info('No implementation lines in this PR. Reporting success.'); + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: context.payload.pull_request.head.sha, + name: 'guardrail/test-ratio', + status: 'completed', + conclusion: 'success', + output: { + title: 'Test-to-code ratio: no implementation changes', + summary: 'This PR contains no implementation line additions. Test ratio check is not applicable.', + }, + }); + return; + } + + // Calculate ratio + const ratio = testLines / implLines; + const passed = ratio >= threshold; + + core.info(`Test-to-code ratio: ${ratio.toFixed(2)} (threshold: ${threshold})`); + + // Check for non-stale PR approval override + const { data: reviews } = await github.rest.pulls.listReviews({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + + const headSha = context.payload.pull_request.head.sha; + const hasValidApproval = hasNonStaleApproval(reviews, headSha); + + // Determine conclusion + let conclusion; + let title; + let summary; + + if (passed) { + conclusion = 'success'; + title = `Test-to-code ratio: ${ratio.toFixed(2)} (threshold: ${threshold})`; + summary = `PR has ${testLines} test lines and ${implLines} implementation lines added. Ratio ${ratio.toFixed(2)} meets the threshold of ${threshold}.`; + } else if (hasValidApproval) { + conclusion = 'neutral'; + title = `Test-to-code ratio: ${ratio.toFixed(2)} — approved by reviewer`; + summary = `PR has ${testLines} test lines and ${implLines} implementation lines added. Ratio ${ratio.toFixed(2)} is below the threshold of ${threshold}, but a non-stale approval exists. Human has accepted the current state.`; + } else { + conclusion = configuredConclusion; + title = `Test-to-code ratio: ${ratio.toFixed(2)} (threshold: ${threshold})`; + summary = `PR has ${testLines} test lines and ${implLines} implementation lines added. Ratio ${ratio.toFixed(2)} is below the threshold of ${threshold}. Add more tests or approve the PR to override.`; + } + + // Build annotations for implementation files lacking test coverage + const annotations = []; + if (!passed && !hasValidApproval) { + for (const file of implFilesWithNoTests) { + if (file.additions > 0) { + annotations.push({ + path: file.filename, + start_line: 1, + end_line: 1, + annotation_level: 'warning', + message: `This implementation file has ${file.additions} added lines. The overall test-to-code ratio (${ratio.toFixed(2)}) is below the threshold (${threshold}). Consider adding corresponding tests.`, + }); + } + } + } + + const truncatedAnnotations = annotations.slice(0, 50); + + // Report check run + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: headSha, + name: 'guardrail/test-ratio', + status: 'completed', + conclusion: conclusion, + output: { + title: title, + summary: summary, + annotations: truncatedAnnotations, + }, + }); + + core.info(`Check run created with conclusion: ${conclusion}`); +}; diff --git a/.github/agent-workflow/scripts/lib/api-patterns.js b/.github/agent-workflow/scripts/lib/api-patterns.js new file mode 100644 index 0000000..afa1a2b --- /dev/null +++ b/.github/agent-workflow/scripts/lib/api-patterns.js @@ -0,0 +1,80 @@ +/** + * Detect API surface changes in a diff + * @param {string} diff - Unified diff content + * @param {string} filename - File being changed + * @returns {string[]} - Array of detected API changes + */ +function detectAPIChanges(diff, filename) { + if (!diff) return []; + + const changes = []; + const lines = diff.split('\n'); + + // Language-specific patterns for API surface detection + const patterns = { + js: [ + /^\+\s*export\s+(function|const|let|var|class|interface|type|enum)\s+(\w+)/, + /^\+\s*export\s+default/, + /^\+\s*export\s*{/ + ], + ts: [ + /^\+\s*export\s+(function|const|let|var|class|interface|type|enum)\s+(\w+)/, + /^\+\s*export\s+default/, + /^\+\s*export\s*{/, + /^\+\s*export\s+interface\s+(\w+)/ + ], + py: [ + /^\+\s*class\s+(\w+)/, + /^\+\s*def\s+(\w+)/, + /^\+\s*async\s+def\s+(\w+)/ + ], + go: [ + /^\+\s*func\s+(\w+)/, + /^\+\s*type\s+(\w+)\s+(?:struct|interface)/ + ], + rs: [ + /^\+\s*pub\s+fn\s+(\w+)/, + /^\+\s*pub\s+struct\s+(\w+)/, + /^\+\s*pub\s+enum\s+(\w+)/, + /^\+\s*pub\s+trait\s+(\w+)/ + ] + }; + + // Determine language from file extension + const ext = filename.split('.').pop(); + const langPatterns = patterns[ext] || patterns.js; + + for (const line of lines) { + // Only look at added lines + if (!line.startsWith('+')) continue; + + for (const pattern of langPatterns) { + const match = line.match(pattern); + if (match) { + const name = match[2] || match[1] || 'exported item'; + changes.push(`Added/modified export: ${name}`); + break; + } + } + } + + // Also check for removed exports + for (const line of lines) { + if (!line.startsWith('-')) continue; + + for (const pattern of langPatterns) { + // Adjust pattern for removal (- instead of +) + const removePattern = new RegExp(pattern.source.replace(/^\\\+/, '-')); + const match = line.match(removePattern); + if (match) { + const name = match[2] || match[1] || 'exported item'; + changes.push(`Removed export: ${name}`); + break; + } + } + } + + return changes; +} + +module.exports = { detectAPIChanges }; diff --git a/.github/agent-workflow/scripts/lib/approval.js b/.github/agent-workflow/scripts/lib/approval.js new file mode 100644 index 0000000..fc84f8b --- /dev/null +++ b/.github/agent-workflow/scripts/lib/approval.js @@ -0,0 +1,15 @@ +/** + * Check if there is a non-stale PR approval + * @param {Array} reviews - Array of review objects from GitHub API + * @param {string} headSha - Current head SHA of the PR + * @returns {boolean} + */ +function hasNonStaleApproval(reviews, headSha) { + if (!reviews || reviews.length === 0) return false; + + return reviews.some( + review => review.state === 'APPROVED' && review.commit_id === headSha + ); +} + +module.exports = { hasNonStaleApproval }; diff --git a/.github/agent-workflow/scripts/lib/file-patterns.js b/.github/agent-workflow/scripts/lib/file-patterns.js new file mode 100644 index 0000000..6f0ccff --- /dev/null +++ b/.github/agent-workflow/scripts/lib/file-patterns.js @@ -0,0 +1,53 @@ +/** + * Check if a file is a test file + * @param {string} filename + * @returns {boolean} + */ +function isTestFile(filename) { + return /\.(test|spec)\.(js|ts|jsx|tsx|py|go|rs)$/.test(filename) || + /__tests__\//.test(filename) || + /(^|\/)tests?\//.test(filename) || + /_test\.(go|rs)$/.test(filename); +} + +/** + * Check if a file is a code file + * @param {string} filename + * @returns {boolean} + */ +function isCodeFile(filename) { + return /\.(js|ts|jsx|tsx|py|go|rs|java|rb|php|c|cpp|h|hpp)$/.test(filename) && + !isTestFile(filename); +} + +/** + * Check if a file is a dependency manifest + * @param {string} filename + * @returns {boolean} + */ +function isDependencyFile(filename) { + const depFiles = [ + 'package.json', + 'package-lock.json', + 'yarn.lock', + 'pnpm-lock.yaml', + 'requirements.txt', + 'Pipfile', + 'Pipfile.lock', + 'go.mod', + 'go.sum', + 'Cargo.toml', + 'Cargo.lock', + 'pom.xml', + 'build.gradle', + 'Gemfile', + 'Gemfile.lock', + 'composer.json', + 'composer.lock' + ]; + + const basename = filename.split('/').pop(); + return depFiles.includes(basename); +} + +module.exports = { isTestFile, isCodeFile, isDependencyFile }; diff --git a/.github/agent-workflow/scripts/lib/fixes-parser.js b/.github/agent-workflow/scripts/lib/fixes-parser.js new file mode 100644 index 0000000..313114e --- /dev/null +++ b/.github/agent-workflow/scripts/lib/fixes-parser.js @@ -0,0 +1,25 @@ +/** + * Parse "Fixes #N" references from PR body + * @param {string} body - PR body text + * @returns {number[]} - Array of issue numbers + */ +function parseFixesReferences(body) { + if (!body) return []; + + const regex = /fixes\s+#(\d+)/gi; + const matches = []; + const seen = new Set(); + + let match; + while ((match = regex.exec(body)) !== null) { + const issueNum = parseInt(match[1], 10); + if (!seen.has(issueNum)) { + matches.push(issueNum); + seen.add(issueNum); + } + } + + return matches; +} + +module.exports = { parseFixesReferences }; diff --git a/.github/agent-workflow/scripts/lib/patch-parser.js b/.github/agent-workflow/scripts/lib/patch-parser.js new file mode 100644 index 0000000..4f8bb82 --- /dev/null +++ b/.github/agent-workflow/scripts/lib/patch-parser.js @@ -0,0 +1,42 @@ +/** + * Parse line numbers of added/modified lines from a unified diff patch + * @param {string} patch - Unified diff patch content + * @returns {number[]} - Array of line numbers that were added or modified + */ +function parseLineNumbers(patch) { + if (!patch) return []; + + const lines = patch.split('\n'); + const lineNumbers = []; + let currentLine = 0; + + for (const line of lines) { + // Parse hunk header: @@ -old_start,old_count +new_start,new_count @@ + const hunkMatch = line.match(/^@@\s+-\d+(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s+@@/); + if (hunkMatch) { + currentLine = parseInt(hunkMatch[1], 10); + continue; + } + + // Skip if we haven't seen a hunk header yet + if (currentLine === 0) continue; + + // Added line (+) + if (line.startsWith('+') && !line.startsWith('+++')) { + lineNumbers.push(currentLine); + currentLine++; + } + // Context line (space) or modified line + else if (line.startsWith(' ')) { + currentLine++; + } + // Deleted line (-) - don't increment current line in new file + else if (line.startsWith('-') && !line.startsWith('---')) { + // Don't increment - this line doesn't exist in new version + } + } + + return lineNumbers; +} + +module.exports = { parseLineNumbers }; diff --git a/.github/agent-workflow/scripts/lib/pr-body.js b/.github/agent-workflow/scripts/lib/pr-body.js new file mode 100644 index 0000000..5157b72 --- /dev/null +++ b/.github/agent-workflow/scripts/lib/pr-body.js @@ -0,0 +1,31 @@ +/** + * Replace or add a section in PR body + * @param {string} body - Current PR body + * @param {string} sectionHeader - Section header (e.g., '## Fixes') + * @param {string} newContent - New content for the section + * @returns {string} - Updated PR body + */ +function replaceSection(body, sectionHeader, newContent) { + if (!body) { + return `${sectionHeader}\n${newContent}\n`; + } + + // Escape special regex characters in header + const escapedHeader = sectionHeader.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + + // Match the section from header to next ## or end of string + const sectionRegex = new RegExp( + `(${escapedHeader}\\n)[\\s\\S]*?(?=\\n## |$)`, + 'i' + ); + + if (sectionRegex.test(body)) { + // Replace existing section + return body.replace(sectionRegex, `$1${newContent}\n`); + } else { + // Append new section + return `${body.trim()}\n\n${sectionHeader}\n${newContent}\n`; + } +} + +module.exports = { replaceSection }; diff --git a/.github/agent-workflow/scripts/lib/scope-matcher.js b/.github/agent-workflow/scripts/lib/scope-matcher.js new file mode 100644 index 0000000..85b3d4c --- /dev/null +++ b/.github/agent-workflow/scripts/lib/scope-matcher.js @@ -0,0 +1,94 @@ +/** + * Extract file paths and glob patterns from issue text + * @param {string} text - Issue body or comment text + * @returns {string[]} - Array of unique file paths and patterns + */ +function extractFilePaths(text) { + if (!text) return []; + + const filePathPatterns = [ + // Backtick-wrapped paths with extension or glob patterns (now includes *) + /`([a-zA-Z0-9_./*-]+\.[a-zA-Z0-9]+)`/g, + // Backtick-wrapped paths with wildcards + /`([a-zA-Z0-9_./*-]+\/\*+[a-zA-Z0-9_./*-]*)`/g, + // Bare paths with at least one slash and an extension (including *) + /(?:^|\s)((?:[a-zA-Z0-9_.*-]+\/)+[a-zA-Z0-9_.*-]+\.[a-zA-Z0-9]+)(?:\s|$|[,;)])/gm, + // Paths starting with ./ or common root dirs (including *) + /(?:^|\s)(\.?(?:src|lib|app|test|tests|spec|pkg|cmd|internal|\.github)\/[a-zA-Z0-9_./*-]+)(?:\s|$|[,;)])/gm + ]; + + const paths = new Set(); + for (const pattern of filePathPatterns) { + pattern.lastIndex = 0; + let match; + while ((match = pattern.exec(text)) !== null) { + const filePath = match[1].replace(/^\//, ''); // strip leading slash + paths.add(filePath); + } + } + + return Array.from(paths); +} + +/** + * Convert a glob pattern to a regular expression + * @param {string} pattern - Glob pattern (supports * and **) + * @returns {RegExp} - Regular expression matching the pattern + */ +function globToRegex(pattern) { + // Escape special regex characters except * and / + let regexStr = pattern.replace(/[.+?^${}()|[\]\\]/g, '\\$&'); + + // Use placeholders to protect our regex patterns from later replacements + const PLACEHOLDER_A = '\x00A\x00'; // For **/ + const PLACEHOLDER_B = '\x00B\x00'; // For /** + const PLACEHOLDER_C = '\x00C\x00'; // For ** + + // Replace **/ with a pattern that matches zero or more path segments + // This allows lib/**/*.js to match both lib/file.js and lib/sub/file.js + regexStr = regexStr.replace(/\*\*\//g, PLACEHOLDER_A); + + // Replace /** at the end with a pattern that matches anything + regexStr = regexStr.replace(/\/\*\*$/g, PLACEHOLDER_B); + + // Replace remaining ** with .* (matches anything including /) + regexStr = regexStr.replace(/\*\*/g, PLACEHOLDER_C); + + // Replace single * with regex that matches anything except / + regexStr = regexStr.replace(/\*/g, '[^/]*'); + + // Now replace placeholders with actual regex patterns + regexStr = regexStr.replace(new RegExp(PLACEHOLDER_A, 'g'), '(?:(?:[^/]+/)*)'); + regexStr = regexStr.replace(new RegExp(PLACEHOLDER_B, 'g'), '(?:/.*)?'); + regexStr = regexStr.replace(new RegExp(PLACEHOLDER_C, 'g'), '.*'); + + // Anchor the pattern to match the full path + return new RegExp('^' + regexStr + '$'); +} + +/** + * Check if a changed file path is within scope + * @param {string} changedPath - Path of changed file + * @param {string[]} scopeFiles - Array of scope file paths/prefixes/patterns + * @returns {boolean} + */ +function isInScope(changedPath, scopeFiles) { + for (const scopePath of scopeFiles) { + // Check if this is a glob pattern (contains * or **) + if (scopePath.includes('*')) { + const regex = globToRegex(scopePath); + if (regex.test(changedPath)) return true; + } else { + // Exact match + if (changedPath === scopePath) return true; + + // Scope entry is a directory prefix + const prefix = scopePath.endsWith('/') ? scopePath : scopePath + '/'; + if (changedPath.startsWith(prefix)) return true; + } + } + + return false; +} + +module.exports = { extractFilePaths, isInScope, globToRegex }; diff --git a/.github/agent-workflow/scripts/lib/severity.js b/.github/agent-workflow/scripts/lib/severity.js new file mode 100644 index 0000000..ed264d2 --- /dev/null +++ b/.github/agent-workflow/scripts/lib/severity.js @@ -0,0 +1,40 @@ +/** + * Detect severity level from review comment text + * @param {string} comment - Review comment text + * @returns {string} - 'blocking', 'should-fix', or 'suggestion' + */ +function detectSeverity(comment) { + if (!comment) return 'should-fix'; + + const lower = comment.toLowerCase(); + + // Blocking keywords + const blockingPatterns = [ + /\bblocking\b/, + /\bcritical\b/, + /\bmust\s+(?:be\s+)?fix/, + /\bmust\s+(?:be\s+)?change/ + ]; + + for (const pattern of blockingPatterns) { + if (pattern.test(lower)) return 'blocking'; + } + + // Suggestion keywords + const suggestionPatterns = [ + /^nit:/, + /\bnit\b/, + /\boptional\b/, + /\bconsider\b/, + /\bsuggestion\b/, + /\bminor\b/ + ]; + + for (const pattern of suggestionPatterns) { + if (pattern.test(lower)) return 'suggestion'; + } + + return 'should-fix'; +} + +module.exports = { detectSeverity }; diff --git a/.github/agent-workflow/scripts/orchestrator-check.js b/.github/agent-workflow/scripts/orchestrator-check.js new file mode 100644 index 0000000..5ebd8f2 --- /dev/null +++ b/.github/agent-workflow/scripts/orchestrator-check.js @@ -0,0 +1,371 @@ +const { parseFixesReferences } = require('./lib/fixes-parser.js'); + +module.exports = async function({ github, context, core }) { + const checkName = 'orchestrator'; + + // Read re-review cycle cap from workflow env + function readCycleCap() { + return parseInt(process.env.RE_REVIEW_CYCLE_CAP, 10) || 3; + } + + // Helper: create check run on a specific SHA + async function createCheckRun(headSha, conclusion, title, summary) { + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: headSha, + name: checkName, + status: 'completed', + conclusion, + output: { title, summary } + }); + } + + // Helper: find PRs referencing a given issue + async function findPRsForIssue(issueNumber) { + const prs = []; + let page = 1; + while (true) { + const resp = await github.rest.pulls.list({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'open', + per_page: 100, + page + }); + if (resp.data.length === 0) break; + for (const pr of resp.data) { + const body = pr.body || ''; + const regex = new RegExp(`[Ff]ixes\\s*#${issueNumber}\\b`); + if (regex.test(body)) { + prs.push(pr); + } + } + if (resp.data.length < 100) break; + page++; + } + return prs; + } + + // Helper: query sub-issues via GraphQL + async function getSubIssues(issueNumber) { + const query = ` + query($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + issue(number: $number) { + subIssues(first: 50) { + nodes { + number + title + state + labels(first: 10) { nodes { name } } + } + } + } + } + } + `; + try { + const result = await github.graphql(query, { + owner: context.repo.owner, + repo: context.repo.repo, + number: issueNumber + }); + return result.repository.issue.subIssues.nodes; + } catch (err) { + console.log(`Warning: GraphQL sub-issues query failed: ${err.message}`); + return []; + } + } + + // Helper: count past review cycles from PR comments + async function countReviewCycles(prNumber) { + const comments = await github.paginate( + github.rest.issues.listComments, + { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + per_page: 100 + } + ); + return comments.filter( + c => c.body && c.body.includes('') + ).length; + } + + // Step 1: Determine the parent issue and PR + let parentIssueNumber; + let prNumber; + let headSha; + + if (context.eventName === 'pull_request') { + // PR synchronize event — parse parent issue from PR body + const pr = context.payload.pull_request; + prNumber = pr.number; + headSha = pr.head.sha; + const issueNumbers = parseFixesReferences(pr.body); + if (issueNumbers.length === 0) { + console.log('No "Fixes #N" in PR body. Skipping orchestrator check.'); + await createCheckRun( + headSha, + 'neutral', + 'Orchestrator: no linked issue', + 'No `Fixes #N` reference found in PR description. Orchestrator check skipped.' + ); + return; + } + parentIssueNumber = issueNumbers[0]; + + } else if (context.eventName === 'issues') { + // Issue event — find the PR that references this issue's parent + const changedIssue = context.payload.issue; + + const openPRs = []; + let page = 1; + while (true) { + const resp = await github.rest.pulls.list({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'open', + per_page: 100, + page + }); + if (resp.data.length === 0) break; + openPRs.push(...resp.data); + if (resp.data.length < 100) break; + page++; + } + + // Build a map of parent issue number -> PR data + const parentToPR = new Map(); + for (const pr of openPRs) { + const issueNumbers = parseFixesReferences(pr.body); + if (issueNumbers.length > 0) { + parentToPR.set(issueNumbers[0], pr); + } + } + + if (parentToPR.size === 0) { + console.log('No open PRs with "Fixes #N" references found. Nothing to do.'); + return; + } + + // Check if the changed issue IS a parent issue referenced by a PR + if (parentToPR.has(changedIssue.number)) { + parentIssueNumber = changedIssue.number; + const pr = parentToPR.get(changedIssue.number); + prNumber = pr.number; + headSha = pr.head.sha; + } else { + // Check if the changed issue is a sub-issue of any parent + let found = false; + for (const [parentNum, pr] of parentToPR.entries()) { + const subIssues = await getSubIssues(parentNum); + const isChild = subIssues.some( + si => si.number === changedIssue.number + ); + if (isChild) { + parentIssueNumber = parentNum; + prNumber = pr.number; + headSha = pr.head.sha; + found = true; + break; + } + } + if (!found) { + console.log( + `Issue #${changedIssue.number} is not a sub-issue of any PR-linked parent. Nothing to do.` + ); + return; + } + } + } else { + console.log(`Unexpected event: ${context.eventName}. Skipping.`); + return; + } + + console.log(`Parent issue: #${parentIssueNumber}, PR: #${prNumber}, HEAD: ${headSha}`); + + // Step 2: Query sub-issues and check for blockers + const subIssues = await getSubIssues(parentIssueNumber); + console.log(`Found ${subIssues.length} sub-issues for #${parentIssueNumber}`); + + const openBlockers = subIssues.filter(si => { + if (si.state !== 'OPEN') return false; + const labels = si.labels.nodes.map(l => l.name); + return labels.includes('blocking'); + }); + + // Step 3: If blockers exist, report failing check + if (openBlockers.length > 0) { + const blockerList = openBlockers + .map(si => `- #${si.number}: ${si.title}`) + .join('\n'); + + await createCheckRun( + headSha, + 'action_required', + `Orchestrator: ${openBlockers.length} blocking issue(s)`, + [ + `PR #${prNumber} cannot merge. Issue #${parentIssueNumber} has open blocking sub-issues:`, + '', + blockerList, + '', + 'Resolve these blocking issues or approve the PR to override.' + ].join('\n') + ); + console.log(`Reported failing check: ${openBlockers.length} blockers.`); + return; + } + + // Step 4: No blockers — assess re-review need + console.log('No open blockers. Assessing re-review need...'); + + const cycleCap = readCycleCap(); + const pastCycles = await countReviewCycles(prNumber); + console.log(`Review cycles so far: ${pastCycles}, cap: ${cycleCap}`); + + // If we've reached the cycle cap, pass without re-review + if (pastCycles >= cycleCap) { + await createCheckRun( + headSha, + 'success', + 'Orchestrator: passing (review cycle cap reached)', + [ + `All blocking sub-issues for #${parentIssueNumber} are resolved.`, + '', + `Re-review cycle cap reached (${pastCycles}/${cycleCap}). No further re-review.`, + 'PR is clear to merge.' + ].join('\n') + ); + console.log('Cycle cap reached. Reporting success.'); + return; + } + + // Assess whether re-review is needed + let needsReReview = false; + + try { + const { data: prData } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber + }); + const baseBranch = prData.base.ref; + + const { data: files } = await github.rest.pulls.listFiles({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + per_page: 100 + }); + + const totalChanges = files.reduce( + (sum, f) => sum + f.additions + f.deletions, 0 + ); + const fileCount = files.length; + const fileList = files + .map(f => `${f.filename} (+${f.additions}/-${f.deletions})`) + .join('\n'); + + if (totalChanges === 0) { + console.log('No changes in PR. Skipping re-review assessment.'); + needsReReview = false; + } else { + const assessPrompt = [ + 'You are assessing whether a PR needs re-review after changes were made to address review findings.', + '', + `PR #${prNumber} targets branch "${baseBranch}" and fixes issue #${parentIssueNumber}.`, + `Past review cycles: ${pastCycles}`, + '', + `The PR modifies ${fileCount} files with ${totalChanges} total line changes:`, + fileList, + '', + 'Based on the scope and nature of these changes, should reviewers re-review this PR?', + 'Consider: Are the changes small and surgical (e.g., null checks, single test additions)?', + 'Or are they broad and structural (e.g., new modules, architectural changes, many files)?', + '', + 'Respond with ONLY one word: YES or NO', + ].join('\n'); + + const { execSync } = require('child_process'); + try { + const result = execSync( + `claude -p "${assessPrompt.replace(/"/g, '\\"')}"`, + { encoding: 'utf-8', timeout: 60000 } + ).trim(); + + console.log(`Claude assessment result: ${result}`); + needsReReview = /\bYES\b/i.test(result); + } catch (claudeErr) { + console.log(`Claude assessment failed: ${claudeErr.message}`); + needsReReview = false; + } + } + } catch (err) { + console.log(`Error during re-review assessment: ${err.message}`); + needsReReview = false; + } + + // Step 5: Trigger re-review or report passing + if (needsReReview) { + console.log('Re-review warranted. Triggering PR Review workflow...'); + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body: [ + '', + `**Orchestrator:** Triggering re-review (cycle ${pastCycles + 1}/${cycleCap}).`, + '', + 'Changes since last review warrant another review pass.' + ].join('\n') + }); + + try { + await github.rest.actions.createWorkflowDispatch({ + owner: context.repo.owner, + repo: context.repo.repo, + workflow_id: 'pr-review.yml', + ref: context.ref || 'main', + inputs: { + 'pr-number': prNumber.toString() + } + }); + console.log('PR Review workflow triggered successfully.'); + } catch (dispatchErr) { + console.log(`Warning: Could not trigger PR Review workflow: ${dispatchErr.message}`); + } + + await createCheckRun( + headSha, + 'neutral', + `Orchestrator: re-review triggered (cycle ${pastCycles + 1}/${cycleCap})`, + [ + `All blocking sub-issues for #${parentIssueNumber} are resolved.`, + '', + `Changes since last review warrant re-review. Cycle ${pastCycles + 1} of ${cycleCap} triggered.`, + 'The PR Review workflow has been dispatched. Orchestrator will re-evaluate after review completes.' + ].join('\n') + ); + } else { + console.log('No re-review needed. Reporting success.'); + + await createCheckRun( + headSha, + 'success', + 'Orchestrator: all clear', + [ + `All blocking sub-issues for #${parentIssueNumber} are resolved.`, + '', + pastCycles > 0 + ? `No further re-review needed after ${pastCycles} review cycle(s).` + : 'No re-review warranted based on change assessment.', + '', + 'PR is clear to merge.' + ].join('\n') + ); + } +}; diff --git a/.github/agent-workflow/scripts/pr-context.js b/.github/agent-workflow/scripts/pr-context.js new file mode 100644 index 0000000..58b1d56 --- /dev/null +++ b/.github/agent-workflow/scripts/pr-context.js @@ -0,0 +1,34 @@ +const { parseFixesReferences } = require('./lib/fixes-parser.js'); + +module.exports = async function({ github, context, core }) { + let prNumber; + let prData; + + if (context.eventName === 'workflow_dispatch') { + // workflow_dispatch input is passed via environment variable + prNumber = parseInt(process.env.PR_NUMBER, 10); + const { data } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + prData = data; + } else { + prNumber = context.payload.pull_request.number; + prData = context.payload.pull_request; + } + + // Parse "fixes #N" or "Fixes #N" from PR body + const body = prData.body || ''; + const issueNumbers = parseFixesReferences(body); + const parentIssue = issueNumbers.length > 0 ? issueNumbers[0].toString() : ''; + + if (!parentIssue) { + console.log('No parent issue found — PR body has no "Fixes #N" reference.'); + } + + core.setOutput('pr-number', prNumber.toString()); + core.setOutput('parent-issue', parentIssue); + core.setOutput('base-branch', prData.base.ref); + core.setOutput('pr-title', prData.title); +}; diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..fb256ce --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,40 @@ +name: CI + +on: + pull_request: + types: [opened, synchronize] + push: + branches: [main] + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: 'lts/*' + cache: 'npm' + + - name: Install dependencies + run: npm ci + + - name: Run tests + run: npm test + + - name: Report test results + if: always() + run: | + echo "Test suite complete" + if [ $? -eq 0 ]; then + echo "✅ All tests passed" + else + echo "❌ Some tests failed" + exit 1 + fi diff --git a/.github/workflows/guardrail-api-surface.yml b/.github/workflows/guardrail-api-surface.yml new file mode 100644 index 0000000..2b425e3 --- /dev/null +++ b/.github/workflows/guardrail-api-surface.yml @@ -0,0 +1,29 @@ +name: "Guardrail: API Surface Changes" + +on: + pull_request: + types: [opened, synchronize] + +permissions: + checks: write + pull-requests: read + contents: read + +env: + CONCLUSION: action_required # action_required | neutral + +jobs: + api-surface-check: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + sparse-checkout: .github/agent-workflow + + - name: Check API surface changes + uses: actions/github-script@v7 + with: + script: | + const run = require('./.github/agent-workflow/scripts/guardrail-api-surface.js'); + await run({ github, context, core }); \ No newline at end of file diff --git a/.github/workflows/guardrail-commits.yml b/.github/workflows/guardrail-commits.yml new file mode 100644 index 0000000..56b77c2 --- /dev/null +++ b/.github/workflows/guardrail-commits.yml @@ -0,0 +1,137 @@ +name: "Guardrail: Commit Messages" + +on: + pull_request: + types: [opened, synchronize] + +permissions: + checks: write + contents: read + pull-requests: read + +env: + CONCLUSION: neutral # action_required | neutral + +jobs: + commit-messages: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 # Need full history for commitlint + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Install commitlint + run: | + npm install -g @commitlint/cli @commitlint/config-conventional + + - name: Validate commit messages + id: commitlint + continue-on-error: true + run: | + set +e # Don't exit on error + + # Get PR commits + BASE_SHA=${{ github.event.pull_request.base.sha }} + HEAD_SHA=${{ github.event.pull_request.head.sha }} + + # Run commitlint and capture output + OUTPUT=$(commitlint --from ${BASE_SHA} --to ${HEAD_SHA} --config .github/agent-workflow/commitlint.config.js 2>&1) + EXIT_CODE=$? + + # Save results for next step + echo "exit_code=${EXIT_CODE}" >> $GITHUB_OUTPUT + + # Save output (escape for multiline) + { + echo "output<> $GITHUB_OUTPUT + + exit ${EXIT_CODE} + + - name: Report results + if: always() + uses: actions/github-script@v7 + with: + script: | + const { hasNonStaleApproval } = require('./.github/agent-workflow/scripts/lib/approval.js'); + const configuredConclusion = process.env.CONCLUSION || 'neutral'; + + // Check for non-stale PR approval override + const reviews = await github.rest.pulls.listReviews({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number + }); + + const headSha = context.payload.pull_request.head.sha; + const hasValidApproval = hasNonStaleApproval(reviews.data, headSha); + + if (hasValidApproval) { + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: context.sha, + name: 'guardrail/commit-messages', + conclusion: 'neutral', + output: { + title: 'Commit message check: approved by reviewer', + summary: 'A non-stale PR approval overrides this guardrail check.' + } + }); + return; + } + + // Get commitlint results + const exitCode = parseInt('${{ steps.commitlint.outputs.exit_code }}'); + const output = `${{ steps.commitlint.outputs.output }}`; + + if (exitCode === 0) { + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: context.sha, + name: 'guardrail/commit-messages', + conclusion: 'success', + output: { + title: 'Commit message check: all commits conform', + summary: 'All commits follow conventional commit format (validated by commitlint).' + } + }); + return; + } + + // Build failure summary + let summary = '## Validation failed\n\n'; + summary += 'Commit messages do not conform to conventional commit format.\n\n'; + summary += '### commitlint output:\n\n'; + summary += '```\n'; + summary += output; + summary += '\n```\n\n'; + summary += '## Expected format\n\n'; + summary += '```\n'; + summary += 'type(optional-scope): description\n'; + summary += '```\n\n'; + summary += 'Valid types: `feat`, `fix`, `chore`, `docs`, `test`, `refactor`, `ci`, `style`, `perf`, `build`, `revert`\n\n'; + summary += 'See: https://www.conventionalcommits.org/\n\n'; + summary += `**Configured conclusion:** \`${configuredConclusion}\`\n\n`; + summary += 'To override: submit an approving PR review. The approval must be on the current head commit to be non-stale.\n'; + + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + head_sha: context.sha, + name: 'guardrail/commit-messages', + conclusion: configuredConclusion, + output: { + title: 'Commit message check: validation failed', + summary: summary + } + }); diff --git a/.github/workflows/guardrail-dependencies.yml b/.github/workflows/guardrail-dependencies.yml new file mode 100644 index 0000000..f43e5a2 --- /dev/null +++ b/.github/workflows/guardrail-dependencies.yml @@ -0,0 +1,28 @@ +name: "Guardrail: Dependency Changes" + +on: + pull_request: + types: [opened, synchronize] + +permissions: + checks: write + contents: read + pull-requests: read + issues: read + +env: + CONCLUSION: action_required # action_required | neutral + +jobs: + dependency-changes: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Check dependency changes + uses: actions/github-script@v7 + with: + script: | + const run = require('./.github/agent-workflow/scripts/guardrail-dependencies.js'); + await run({ github, context, core }); \ No newline at end of file diff --git a/.github/workflows/guardrail-scope.yml b/.github/workflows/guardrail-scope.yml new file mode 100644 index 0000000..c496e18 --- /dev/null +++ b/.github/workflows/guardrail-scope.yml @@ -0,0 +1,30 @@ +name: "Guardrail: Scope Enforcement" + +on: + pull_request: + types: [opened, synchronize] + +permissions: + checks: write + issues: read + pull-requests: read + contents: read + +env: + CONCLUSION: action_required # action_required | neutral + +jobs: + scope-check: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + sparse-checkout: .github/agent-workflow + + - name: Check scope enforcement + uses: actions/github-script@v7 + with: + script: | + const run = require('./.github/agent-workflow/scripts/guardrail-scope.js'); + await run({ github, context, core }); diff --git a/.github/workflows/guardrail-test-ratio.yml b/.github/workflows/guardrail-test-ratio.yml new file mode 100644 index 0000000..731fd0d --- /dev/null +++ b/.github/workflows/guardrail-test-ratio.yml @@ -0,0 +1,28 @@ +name: "Guardrail: Test-to-Code Ratio" + +on: + pull_request: + types: [opened, synchronize] + +permissions: + checks: write + pull-requests: read + contents: read + +env: + CONCLUSION: action_required # action_required | neutral + THRESHOLD: "0.5" # minimum test-to-code line ratio + +jobs: + test-ratio-check: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Check test-to-code ratio + uses: actions/github-script@v7 + with: + script: | + const run = require('./.github/agent-workflow/scripts/guardrail-test-ratio.js'); + await run({ github, context, core }); diff --git a/.github/workflows/orchestrator-check.yml b/.github/workflows/orchestrator-check.yml new file mode 100644 index 0000000..d8de910 --- /dev/null +++ b/.github/workflows/orchestrator-check.yml @@ -0,0 +1,36 @@ +name: Orchestrator Status Check + +"on": + issues: + types: [opened, closed, labeled, unlabeled] + pull_request: + types: [synchronize] + +permissions: + checks: write + issues: read + pull-requests: read + contents: read + actions: write + +env: + RE_REVIEW_CYCLE_CAP: "3" # max re-review cycles before auto-passing + +jobs: + orchestrator: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + sparse-checkout: .github/agent-workflow + fetch-depth: 0 + + - name: Orchestrator check + uses: actions/github-script@v7 + env: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + with: + script: | + const run = require('./.github/agent-workflow/scripts/orchestrator-check.js'); + await run({ github, context, core }); \ No newline at end of file diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml new file mode 100644 index 0000000..cbdb8d0 --- /dev/null +++ b/.github/workflows/pr-review.yml @@ -0,0 +1,120 @@ +name: PR Review + +"on": + pull_request: + types: [opened, synchronize] + workflow_dispatch: + inputs: + pr-number: + description: "PR number to review (used by orchestrator for re-review)" + required: true + type: number + +permissions: + contents: read + issues: write + pull-requests: write + id-token: write + +jobs: + # ── Resolve context shared by all reviewers ────────────────────────── + resolve-context: + runs-on: ubuntu-latest + outputs: + pr-number: ${{ steps.ctx.outputs.pr-number }} + parent-issue: ${{ steps.ctx.outputs.parent-issue }} + base-branch: ${{ steps.ctx.outputs.base-branch }} + pr-title: ${{ steps.ctx.outputs.pr-title }} + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + sparse-checkout: .github/agent-workflow + + - name: Resolve PR context + id: ctx + uses: actions/github-script@v7 + env: + PR_NUMBER: ${{ inputs.pr-number }} + with: + script: | + const run = require('./.github/agent-workflow/scripts/pr-context.js'); + await run({ github, context, core }); + + # ── Correctness Reviewer ───────────────────────────────────────────── + review-correctness: + needs: resolve-context + if: needs.resolve-context.outputs.parent-issue != '' + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Run correctness review + uses: anthropics/claude-code-action@v1 + with: + prompt: | + Read and follow .claude/skills/reviewer-correctness/SKILL.md + + Context: + - Base branch: origin/${{ needs.resolve-context.outputs.base-branch }} + - Parent issue: #${{ needs.resolve-context.outputs.parent-issue }} + + File findings as sub-issues of #${{ needs.resolve-context.outputs.parent-issue }}. + See .claude/skills/github-issues/SKILL.md for the GraphQL patterns. + env: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + + # ── Tests Reviewer ─────────────────────────────────────────────────── + review-tests: + needs: resolve-context + if: needs.resolve-context.outputs.parent-issue != '' + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Run tests review + uses: anthropics/claude-code-action@v1 + with: + prompt: | + Read and follow .claude/skills/reviewer-tests/SKILL.md + + Context: + - Base branch: origin/${{ needs.resolve-context.outputs.base-branch }} + - Parent issue: #${{ needs.resolve-context.outputs.parent-issue }} + + File findings as sub-issues of #${{ needs.resolve-context.outputs.parent-issue }}. + See .claude/skills/github-issues/SKILL.md for the GraphQL patterns. + env: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + + # ── Architecture Reviewer ──────────────────────────────────────────── + review-architecture: + needs: resolve-context + if: needs.resolve-context.outputs.parent-issue != '' + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Run architecture review + uses: anthropics/claude-code-action@v1 + with: + prompt: | + Read and follow .claude/skills/reviewer-architecture/SKILL.md + + Context: + - Base branch: origin/${{ needs.resolve-context.outputs.base-branch }} + - Parent issue: #${{ needs.resolve-context.outputs.parent-issue }} + + File findings as sub-issues of #${{ needs.resolve-context.outputs.parent-issue }}. + See .claude/skills/github-issues/SKILL.md for the GraphQL patterns. + env: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..97faa46 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +tests/__pycache__/ +node_modules/ diff --git a/README.md b/README.md index 9e92754..9aef7c7 100644 --- a/README.md +++ b/README.md @@ -82,30 +82,25 @@ See [docs/design.md](docs/design.md) for the full design document. ## Configuration -After installation, configure guardrails in `.github/agent-workflow/config.yaml`: +Each guardrail is a standalone workflow file in `.github/workflows/`. Configure by editing the `env:` block at the top of each workflow: ```yaml -re-review-cycle-cap: 3 - -guardrails: - scope-enforcement: - enabled: true - conclusion: action_required - test-ratio: - enabled: true - conclusion: action_required - threshold: 0.5 - dependency-changes: - enabled: true - conclusion: action_required - api-surface: - enabled: true - conclusion: action_required - commit-messages: - enabled: true - conclusion: neutral # warning only +# guardrail-scope.yml +env: + CONCLUSION: action_required # action_required | neutral + +# guardrail-test-ratio.yml +env: + CONCLUSION: action_required + THRESHOLD: "0.5" # minimum test-to-code line ratio + +# orchestrator-check.yml +env: + RE_REVIEW_CYCLE_CAP: "3" # max re-review cycles before auto-passing ``` +To disable a guardrail, delete its workflow file. + ## Requirements - GitHub repository diff --git a/docs/design.md b/docs/design.md index 0165f1b..d91935f 100644 --- a/docs/design.md +++ b/docs/design.md @@ -454,7 +454,7 @@ agent-workflow-template/ │ │ ├── guardrail-api-surface.yml # API surface change detection (native check run) │ │ └── guardrail-commits.yml # Commit message structure (native check run) │ ├── agent-workflow/ -│ │ └── config.yaml # Workflow configuration (re-review cap, check thresholds, etc.) +│ │ └── scripts/ # Guardrail and orchestrator scripts (used by workflows) │ └── ISSUE_TEMPLATE/ │ ├── task.yml # Structured task template for agent consumption │ └── review-finding.yml # Template for reviewer-created issues diff --git a/package-lock.json b/package-lock.json new file mode 100644 index 0000000..8f72601 --- /dev/null +++ b/package-lock.json @@ -0,0 +1,35 @@ +{ + "name": "agent-workflow-scripts", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "agent-workflow-scripts", + "version": "1.0.0", + "devDependencies": { + "js-yaml": "^4.1.0" + } + }, + "node_modules/argparse": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", + "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", + "dev": true, + "license": "Python-2.0" + }, + "node_modules/js-yaml": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.1.tgz", + "integrity": "sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA==", + "dev": true, + "license": "MIT", + "dependencies": { + "argparse": "^2.0.1" + }, + "bin": { + "js-yaml": "bin/js-yaml.js" + } + } + } +} diff --git a/package.json b/package.json new file mode 100644 index 0000000..f538d2f --- /dev/null +++ b/package.json @@ -0,0 +1,12 @@ +{ + "name": "agent-workflow-scripts", + "version": "1.0.0", + "description": "Shared libraries for GitHub Actions workflows", + "type": "commonjs", + "scripts": { + "test": "node --test" + }, + "devDependencies": { + "js-yaml": "^4.1.0" + } +} diff --git a/tests/lib/api-patterns.test.js b/tests/lib/api-patterns.test.js new file mode 100644 index 0000000..0fbb0b7 --- /dev/null +++ b/tests/lib/api-patterns.test.js @@ -0,0 +1,50 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const { detectAPIChanges } = require('../../.github/agent-workflow/scripts/lib/api-patterns.js'); + +test('detectAPIChanges - JavaScript export changes', () => { + const diff = ` ++export function newAPI() {} +-export function oldAPI() {} ++export const API_CONSTANT = 42; + `; + const changes = detectAPIChanges(diff, 'file.js'); + assert.ok(changes.length > 0); + assert.ok(changes.some(c => c.includes('newAPI'))); +}); + +test('detectAPIChanges - TypeScript interface changes', () => { + const diff = ` ++export interface NewInterface { ++ field: string; ++} + `; + const changes = detectAPIChanges(diff, 'types.ts'); + assert.ok(changes.length > 0); + assert.ok(changes.some(c => c.includes('NewInterface'))); +}); + +test('detectAPIChanges - Python class changes', () => { + const diff = ` ++class PublicAPI: ++ def __init__(self): ++ pass + `; + const changes = detectAPIChanges(diff, 'module.py'); + assert.ok(changes.length > 0); +}); + +test('detectAPIChanges - no API changes', () => { + const diff = ` ++// Internal helper function ++function helper() {} ++const internal = 123; + `; + const changes = detectAPIChanges(diff, 'file.js'); + assert.strictEqual(changes.length, 0); +}); + +test('detectAPIChanges - empty diff', () => { + const changes = detectAPIChanges('', 'file.js'); + assert.strictEqual(changes.length, 0); +}); diff --git a/tests/lib/approval.test.js b/tests/lib/approval.test.js new file mode 100644 index 0000000..bdc9c56 --- /dev/null +++ b/tests/lib/approval.test.js @@ -0,0 +1,38 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const { hasNonStaleApproval } = require('../../.github/agent-workflow/scripts/lib/approval.js'); + +test('hasNonStaleApproval - approved at head SHA', () => { + const reviews = [ + { state: 'APPROVED', commit_id: 'abc123' }, + { state: 'COMMENTED', commit_id: 'abc123' } + ]; + assert.strictEqual(hasNonStaleApproval(reviews, 'abc123'), true); +}); + +test('hasNonStaleApproval - no approvals', () => { + const reviews = [ + { state: 'COMMENTED', commit_id: 'abc123' }, + { state: 'CHANGES_REQUESTED', commit_id: 'abc123' } + ]; + assert.strictEqual(hasNonStaleApproval(reviews, 'abc123'), false); +}); + +test('hasNonStaleApproval - stale approval (different SHA)', () => { + const reviews = [ + { state: 'APPROVED', commit_id: 'old123' } + ]; + assert.strictEqual(hasNonStaleApproval(reviews, 'new456'), false); +}); + +test('hasNonStaleApproval - multiple approvals, at least one non-stale', () => { + const reviews = [ + { state: 'APPROVED', commit_id: 'old123' }, + { state: 'APPROVED', commit_id: 'new456' } + ]; + assert.strictEqual(hasNonStaleApproval(reviews, 'new456'), true); +}); + +test('hasNonStaleApproval - empty reviews', () => { + assert.strictEqual(hasNonStaleApproval([], 'abc123'), false); +}); diff --git a/tests/lib/file-patterns.test.js b/tests/lib/file-patterns.test.js new file mode 100644 index 0000000..89134c3 --- /dev/null +++ b/tests/lib/file-patterns.test.js @@ -0,0 +1,51 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const { isTestFile, isCodeFile, isDependencyFile } = require('../../.github/agent-workflow/scripts/lib/file-patterns.js'); + +test('isTestFile - recognizes test files', () => { + assert.strictEqual(isTestFile('src/foo.test.js'), true); + assert.strictEqual(isTestFile('src/foo.spec.ts'), true); + assert.strictEqual(isTestFile('__tests__/foo.js'), true); + assert.strictEqual(isTestFile('tests/integration/bar.py'), true); + assert.strictEqual(isTestFile('lib/foo_test.go'), true); +}); + +test('isTestFile - rejects non-test files', () => { + assert.strictEqual(isTestFile('src/index.js'), false); + assert.strictEqual(isTestFile('lib/utils.ts'), false); + assert.strictEqual(isTestFile('README.md'), false); +}); + +test('isCodeFile - recognizes code files', () => { + assert.strictEqual(isCodeFile('src/index.js'), true); + assert.strictEqual(isCodeFile('lib/utils.ts'), true); + assert.strictEqual(isCodeFile('app/main.py'), true); + assert.strictEqual(isCodeFile('cmd/server.go'), true); + assert.strictEqual(isCodeFile('pkg/auth/handler.rs'), true); +}); + +test('isCodeFile - rejects non-code files', () => { + assert.strictEqual(isCodeFile('README.md'), false); + assert.strictEqual(isCodeFile('package.json'), false); + assert.strictEqual(isCodeFile('.gitignore'), false); + assert.strictEqual(isCodeFile('docs/guide.txt'), false); +}); + +test('isDependencyFile - recognizes dependency files', () => { + assert.strictEqual(isDependencyFile('package.json'), true); + assert.strictEqual(isDependencyFile('package-lock.json'), true); + assert.strictEqual(isDependencyFile('requirements.txt'), true); + assert.strictEqual(isDependencyFile('go.mod'), true); + assert.strictEqual(isDependencyFile('go.sum'), true); + assert.strictEqual(isDependencyFile('Cargo.toml'), true); + assert.strictEqual(isDependencyFile('Cargo.lock'), true); + assert.strictEqual(isDependencyFile('pom.xml'), true); + assert.strictEqual(isDependencyFile('Gemfile'), true); + assert.strictEqual(isDependencyFile('Gemfile.lock'), true); +}); + +test('isDependencyFile - rejects non-dependency files', () => { + assert.strictEqual(isDependencyFile('src/index.js'), false); + assert.strictEqual(isDependencyFile('README.md'), false); + assert.strictEqual(isDependencyFile('config.json'), false); +}); diff --git a/tests/lib/fixes-parser.test.js b/tests/lib/fixes-parser.test.js new file mode 100644 index 0000000..00ee044 --- /dev/null +++ b/tests/lib/fixes-parser.test.js @@ -0,0 +1,39 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const { parseFixesReferences } = require('../../.github/agent-workflow/scripts/lib/fixes-parser.js'); + +test('parseFixesReferences - single fixes reference', () => { + const body = 'This PR fixes #123'; + const result = parseFixesReferences(body); + assert.deepStrictEqual(result, [123]); +}); + +test('parseFixesReferences - multiple fixes references', () => { + const body = 'Fixes #10\nFixes #20\nFixes #30'; + const result = parseFixesReferences(body); + assert.deepStrictEqual(result, [10, 20, 30]); +}); + +test('parseFixesReferences - case insensitive', () => { + const body = 'fixes #1\nFIXES #2\nFiXeS #3'; + const result = parseFixesReferences(body); + assert.deepStrictEqual(result, [1, 2, 3]); +}); + +test('parseFixesReferences - no matches', () => { + const body = 'This PR does not fix anything'; + const result = parseFixesReferences(body); + assert.deepStrictEqual(result, []); +}); + +test('parseFixesReferences - null or undefined body', () => { + assert.deepStrictEqual(parseFixesReferences(null), []); + assert.deepStrictEqual(parseFixesReferences(undefined), []); + assert.deepStrictEqual(parseFixesReferences(''), []); +}); + +test('parseFixesReferences - deduplicates issue numbers', () => { + const body = 'Fixes #10\nFixes #10\nFixes #20'; + const result = parseFixesReferences(body); + assert.deepStrictEqual(result, [10, 20]); +}); diff --git a/tests/lib/patch-parser.test.js b/tests/lib/patch-parser.test.js new file mode 100644 index 0000000..d9f570f --- /dev/null +++ b/tests/lib/patch-parser.test.js @@ -0,0 +1,46 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const { parseLineNumbers } = require('../../.github/agent-workflow/scripts/lib/patch-parser.js'); + +test('parseLineNumbers - simple addition', () => { + const patch = ` +@@ -10,5 +10,6 @@ + line 10 + line 11 ++added line 12 + line 13 + `; + const lines = parseLineNumbers(patch); + assert.ok(lines.includes(12)); +}); + +test('parseLineNumbers - multiple hunks', () => { + const patch = ` +@@ -10,3 +10,4 @@ + line 10 ++added line 11 + line 12 +@@ -20,2 +21,3 @@ + line 21 ++added line 22 + `; + const lines = parseLineNumbers(patch); + assert.ok(lines.includes(11)); + assert.ok(lines.includes(22)); +}); + +test('parseLineNumbers - deletions not included', () => { + const patch = ` +@@ -10,4 +10,3 @@ + line 10 +-deleted line 11 + line 12 + `; + const lines = parseLineNumbers(patch); + assert.ok(!lines.includes(11)); +}); + +test('parseLineNumbers - empty patch', () => { + const lines = parseLineNumbers(''); + assert.deepStrictEqual(lines, []); +}); diff --git a/tests/lib/pr-body.test.js b/tests/lib/pr-body.test.js new file mode 100644 index 0000000..7d41d10 --- /dev/null +++ b/tests/lib/pr-body.test.js @@ -0,0 +1,36 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const { replaceSection } = require('../../.github/agent-workflow/scripts/lib/pr-body.js'); + +test('replaceSection - adds new section to empty body', () => { + const result = replaceSection('', '## Fixes', 'Fixes #123'); + assert.ok(result.includes('## Fixes')); + assert.ok(result.includes('Fixes #123')); +}); + +test('replaceSection - replaces existing section', () => { + const body = '## Summary\nSome text\n\n## Fixes\nFixes #100\n\n## Other\nMore text'; + const result = replaceSection(body, '## Fixes', 'Fixes #200\nFixes #201'); + assert.ok(result.includes('Fixes #200')); + assert.ok(result.includes('Fixes #201')); + assert.ok(!result.includes('Fixes #100')); + assert.ok(result.includes('## Summary')); + assert.ok(result.includes('## Other')); +}); + +test('replaceSection - preserves other sections', () => { + const body = '## Summary\nOriginal\n\n## Test Plan\nOriginal test plan'; + const result = replaceSection(body, '## Fixes', 'Fixes #999'); + assert.ok(result.includes('## Summary')); + assert.ok(result.includes('Original')); + assert.ok(result.includes('## Test Plan')); + assert.ok(result.includes('Original test plan')); + assert.ok(result.includes('Fixes #999')); +}); + +test('replaceSection - idempotent updates', () => { + const body = '## Summary\nText\n\n## Fixes\nFixes #123'; + const result1 = replaceSection(body, '## Fixes', 'Fixes #123'); + const result2 = replaceSection(result1, '## Fixes', 'Fixes #123'); + assert.strictEqual(result1, result2); +}); diff --git a/tests/lib/scope-matcher.test.js b/tests/lib/scope-matcher.test.js new file mode 100644 index 0000000..8b088a9 --- /dev/null +++ b/tests/lib/scope-matcher.test.js @@ -0,0 +1,103 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const { extractFilePaths, isInScope, globToRegex } = require('../../.github/agent-workflow/scripts/lib/scope-matcher.js'); + +test('extractFilePaths - backtick-wrapped paths', () => { + const text = 'Modify `src/index.js` and `lib/utils.ts`'; + const paths = extractFilePaths(text); + assert.deepStrictEqual(paths, ['src/index.js', 'lib/utils.ts']); +}); + +test('extractFilePaths - bare paths with slashes', () => { + const text = 'Files: src/app/main.py and lib/helper.go'; + const paths = extractFilePaths(text); + assert.ok(paths.includes('src/app/main.py')); + assert.ok(paths.includes('lib/helper.go')); +}); + +test('extractFilePaths - common root directories', () => { + const text = 'Update src/index.js, test/unit.js, and .github/workflows/ci.yml'; + const paths = extractFilePaths(text); + assert.ok(paths.includes('src/index.js')); + assert.ok(paths.includes('test/unit.js')); + assert.ok(paths.includes('.github/workflows/ci.yml')); +}); + +test('extractFilePaths - no duplicates', () => { + const text = 'File `src/index.js` and src/index.js again'; + const paths = extractFilePaths(text); + assert.strictEqual(paths.filter(p => p === 'src/index.js').length, 1); +}); + +test('extractFilePaths - empty text', () => { + assert.deepStrictEqual(extractFilePaths(''), []); + assert.deepStrictEqual(extractFilePaths(null), []); + assert.deepStrictEqual(extractFilePaths(undefined), []); +}); + +test('isInScope - exact match', () => { + const scopeFiles = ['src/index.js', 'lib/utils.ts']; + assert.strictEqual(isInScope('src/index.js', scopeFiles), true); + assert.strictEqual(isInScope('lib/utils.ts', scopeFiles), true); + assert.strictEqual(isInScope('other/file.js', scopeFiles), false); +}); + +test('isInScope - directory prefix', () => { + const scopeFiles = ['src/', 'lib/auth/']; + assert.strictEqual(isInScope('src/index.js', scopeFiles), true); + assert.strictEqual(isInScope('src/app/main.js', scopeFiles), true); + assert.strictEqual(isInScope('lib/auth/handler.js', scopeFiles), true); + assert.strictEqual(isInScope('lib/other/file.js', scopeFiles), false); +}); + +test('isInScope - prefix without trailing slash', () => { + const scopeFiles = ['src/auth']; + assert.strictEqual(isInScope('src/auth/handler.js', scopeFiles), true); + assert.strictEqual(isInScope('src/auth.ts', scopeFiles), false); +}); + +test('extractFilePaths - glob patterns', () => { + const text = 'Files: `lib/*.js` and `.github/agent-workflow/scripts/lib/*.test.js`'; + const paths = extractFilePaths(text); + assert.ok(paths.includes('lib/*.js')); + assert.ok(paths.includes('.github/agent-workflow/scripts/lib/*.test.js')); +}); + +test('globToRegex - single wildcard', () => { + const regex = globToRegex('lib/*.js'); + assert.strictEqual(regex.test('lib/utils.js'), true); + assert.strictEqual(regex.test('lib/helper.js'), true); + assert.strictEqual(regex.test('lib/sub/utils.js'), false); // * doesn't match / + assert.strictEqual(regex.test('other/utils.js'), false); +}); + +test('globToRegex - double wildcard', () => { + const regex = globToRegex('lib/**/*.js'); + assert.strictEqual(regex.test('lib/utils.js'), true); + assert.strictEqual(regex.test('lib/sub/utils.js'), true); + assert.strictEqual(regex.test('lib/sub/deep/utils.js'), true); + assert.strictEqual(regex.test('other/utils.js'), false); +}); + +test('globToRegex - wildcard in filename', () => { + const regex = globToRegex('lib/*.test.js'); + assert.strictEqual(regex.test('lib/utils.test.js'), true); + assert.strictEqual(regex.test('lib/helper.test.js'), true); + assert.strictEqual(regex.test('lib/utils.js'), false); +}); + +test('isInScope - glob pattern matching', () => { + const scopeFiles = ['lib/*.js', '.github/workflows/*.yml']; + assert.strictEqual(isInScope('lib/utils.js', scopeFiles), true); + assert.strictEqual(isInScope('lib/helper.js', scopeFiles), true); + assert.strictEqual(isInScope('.github/workflows/ci.yml', scopeFiles), true); + assert.strictEqual(isInScope('lib/sub/utils.js', scopeFiles), false); + assert.strictEqual(isInScope('other/file.js', scopeFiles), false); +}); + +test('isInScope - double wildcard pattern', () => { + const scopeFiles = ['.github/agent-workflow/**/*.js']; + assert.strictEqual(isInScope('.github/agent-workflow/scripts/lib/config.js', scopeFiles), true); + assert.strictEqual(isInScope('.github/agent-workflow/scripts/main.js', scopeFiles), true); + assert.strictEqual(isInScope('.github/workflows/ci.yml', scopeFiles), false); +}); diff --git a/tests/lib/severity.test.js b/tests/lib/severity.test.js new file mode 100644 index 0000000..2f2db0d --- /dev/null +++ b/tests/lib/severity.test.js @@ -0,0 +1,28 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const { detectSeverity } = require('../../.github/agent-workflow/scripts/lib/severity.js'); + +test('detectSeverity - blocking keywords', () => { + assert.strictEqual(detectSeverity('This is blocking the release'), 'blocking'); + assert.strictEqual(detectSeverity('BLOCKING: critical issue'), 'blocking'); + assert.strictEqual(detectSeverity('This must be fixed'), 'blocking'); + assert.strictEqual(detectSeverity('critical bug here'), 'blocking'); +}); + +test('detectSeverity - suggestion keywords', () => { + assert.strictEqual(detectSeverity('Consider refactoring this'), 'suggestion'); + assert.strictEqual(detectSeverity('nit: extra space'), 'suggestion'); + assert.strictEqual(detectSeverity('optional: could improve'), 'suggestion'); + assert.strictEqual(detectSeverity('minor: formatting'), 'suggestion'); +}); + +test('detectSeverity - default should-fix', () => { + assert.strictEqual(detectSeverity('This needs to be fixed'), 'should-fix'); + assert.strictEqual(detectSeverity('Bug in the implementation'), 'should-fix'); + assert.strictEqual(detectSeverity('Random comment'), 'should-fix'); +}); + +test('detectSeverity - case insensitive', () => { + assert.strictEqual(detectSeverity('BLOCKING issue'), 'blocking'); + assert.strictEqual(detectSeverity('Nit: formatting'), 'suggestion'); +}); diff --git a/tests/validate-workflows.js b/tests/validate-workflows.js new file mode 100644 index 0000000..204c15c --- /dev/null +++ b/tests/validate-workflows.js @@ -0,0 +1,225 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); +const yaml = require('js-yaml'); + +const workflowsDir = path.join(__dirname, '../.github/workflows'); + +// Get all workflow files +const workflowFiles = fs.readdirSync(workflowsDir) + .filter(f => f.endsWith('.yml') && f !== '.gitkeep') + .map(f => path.join(workflowsDir, f)); + +// Expected workflow files +const expectedWorkflows = [ + 'guardrail-api-surface.yml', + 'guardrail-commits.yml', + 'guardrail-dependencies.yml', + 'guardrail-scope.yml', + 'guardrail-test-ratio.yml', + 'human-review.yml', + 'orchestrator-check.yml', + 'pr-review.yml' +]; + +test('All expected workflow files exist', () => { + const actualFiles = fs.readdirSync(workflowsDir) + .filter(f => f.endsWith('.yml') && f !== '.gitkeep'); + + for (const expected of expectedWorkflows) { + assert.ok(actualFiles.includes(expected), `Missing workflow: ${expected}`); + } +}); + +test('All workflow files are valid YAML', () => { + for (const file of workflowFiles) { + const content = fs.readFileSync(file, 'utf8'); + + // Should not throw + const parsed = yaml.load(content); + assert.ok(parsed, `Failed to parse ${path.basename(file)}`); + } +}); + +test('All workflows have required top-level fields', () => { + for (const file of workflowFiles) { + const content = fs.readFileSync(file, 'utf8'); + const workflow = yaml.load(content); + const name = path.basename(file); + + assert.ok(workflow.name, `${name}: missing 'name' field`); + assert.ok(workflow.on, `${name}: missing 'on' field`); + assert.ok(workflow.permissions, `${name}: missing 'permissions' field`); + assert.ok(workflow.jobs, `${name}: missing 'jobs' field`); + } +}); + +test('All workflows have at least one job', () => { + for (const file of workflowFiles) { + const content = fs.readFileSync(file, 'utf8'); + const workflow = yaml.load(content); + const name = path.basename(file); + + const jobNames = Object.keys(workflow.jobs); + assert.ok(jobNames.length > 0, `${name}: no jobs defined`); + } +}); + +test('All jobs have required fields', () => { + for (const file of workflowFiles) { + const content = fs.readFileSync(file, 'utf8'); + const workflow = yaml.load(content); + const workflowName = path.basename(file); + + for (const [jobName, job] of Object.entries(workflow.jobs)) { + assert.ok(job['runs-on'], `${workflowName}:${jobName}: missing 'runs-on' field`); + assert.ok(job.steps, `${workflowName}:${jobName}: missing 'steps' field`); + assert.ok(Array.isArray(job.steps), `${workflowName}:${jobName}: 'steps' must be an array`); + assert.ok(job.steps.length > 0, `${workflowName}:${jobName}: must have at least one step`); + } + } +}); + +test('Guardrail workflows have correct structure', () => { + const guardrailFiles = workflowFiles.filter(f => path.basename(f).startsWith('guardrail-')); + + for (const file of guardrailFiles) { + const content = fs.readFileSync(file, 'utf8'); + const workflow = yaml.load(content); + const name = path.basename(file); + + // Check trigger + assert.ok(workflow.on.pull_request, `${name}: should trigger on pull_request`); + + // Check permissions + assert.ok(workflow.permissions.checks === 'write', `${name}: should have checks: write permission`); + assert.ok(workflow.permissions.contents === 'read', `${name}: should have contents: read permission`); + assert.ok(workflow.permissions['pull-requests'] === 'read', `${name}: should have pull-requests: read permission`); + + // Check that job uses github-script action + const jobs = Object.values(workflow.jobs); + const hasGithubScript = jobs.some(job => + job.steps.some(step => step.uses && step.uses.includes('actions/github-script')) + ); + assert.ok(hasGithubScript, `${name}: should use actions/github-script`); + } +}); + +test('PR review workflow has correct structure', () => { + const file = workflowFiles.find(f => path.basename(f) === 'pr-review.yml'); + assert.ok(file, 'pr-review.yml not found'); + + const content = fs.readFileSync(file, 'utf8'); + const workflow = yaml.load(content); + + // Check triggers + assert.ok(workflow.on.pull_request, 'should trigger on pull_request'); + assert.ok(workflow.on.workflow_dispatch, 'should support workflow_dispatch'); + + // Check permissions + assert.ok(workflow.permissions.contents === 'read', 'should have contents: read'); + assert.ok(workflow.permissions.issues === 'write', 'should have issues: write'); + assert.ok(workflow.permissions['pull-requests'] === 'write', 'should have pull-requests: write'); + assert.ok(workflow.permissions['id-token'] === 'write', 'should have id-token: write'); + + // Check for resolve-context job + assert.ok(workflow.jobs['resolve-context'], 'should have resolve-context job'); + + // Check for reviewer jobs + assert.ok(workflow.jobs['review-correctness'], 'should have review-correctness job'); + assert.ok(workflow.jobs['review-tests'], 'should have review-tests job'); + assert.ok(workflow.jobs['review-architecture'], 'should have review-architecture job'); + + // Check job dependencies + assert.ok(workflow.jobs['review-correctness'].needs === 'resolve-context', 'review-correctness should depend on resolve-context'); + assert.ok(workflow.jobs['review-tests'].needs === 'resolve-context', 'review-tests should depend on resolve-context'); + assert.ok(workflow.jobs['review-architecture'].needs === 'resolve-context', 'review-architecture should depend on resolve-context'); +}); + +test('Orchestrator workflow has correct structure', () => { + const file = workflowFiles.find(f => path.basename(f) === 'orchestrator-check.yml'); + assert.ok(file, 'orchestrator-check.yml not found'); + + const content = fs.readFileSync(file, 'utf8'); + const workflow = yaml.load(content); + + // Check triggers + assert.ok(workflow.on.issues, 'should trigger on issues events'); + assert.ok(workflow.on.pull_request, 'should trigger on pull_request events'); + + // Check permissions + assert.ok(workflow.permissions.checks === 'write', 'should have checks: write'); + assert.ok(workflow.permissions.issues === 'read', 'should have issues: read'); + assert.ok(workflow.permissions['pull-requests'] === 'read', 'should have pull-requests: read'); + assert.ok(workflow.permissions.contents === 'read', 'should have contents: read'); + assert.ok(workflow.permissions.actions === 'write', 'should have actions: write'); + + // Check for orchestrator job + assert.ok(workflow.jobs.orchestrator, 'should have orchestrator job'); + + // Check for CLAUDE_CODE_OAUTH_TOKEN in env + const orchestratorJob = workflow.jobs.orchestrator; + const hasTokenEnv = orchestratorJob.steps.some(step => + step.env && step.env.CLAUDE_CODE_OAUTH_TOKEN + ); + assert.ok(hasTokenEnv, 'should have CLAUDE_CODE_OAUTH_TOKEN in env'); +}); + +test('Human review workflow has correct structure', () => { + const file = workflowFiles.find(f => path.basename(f) === 'human-review.yml'); + assert.ok(file, 'human-review.yml not found'); + + const content = fs.readFileSync(file, 'utf8'); + const workflow = yaml.load(content); + + // Check trigger + assert.ok(workflow.on.pull_request_review, 'should trigger on pull_request_review'); + + // Check permissions + assert.ok(workflow.permissions.issues === 'write', 'should have issues: write'); + assert.ok(workflow.permissions.contents === 'read', 'should have contents: read'); + assert.ok(workflow.permissions['pull-requests'] === 'write', 'should have pull-requests: write'); +}); + +test('All workflow steps have names', () => { + for (const file of workflowFiles) { + const content = fs.readFileSync(file, 'utf8'); + const workflow = yaml.load(content); + const workflowName = path.basename(file); + + for (const [jobName, job] of Object.entries(workflow.jobs)) { + for (let i = 0; i < job.steps.length; i++) { + const step = job.steps[i]; + assert.ok(step.name, `${workflowName}:${jobName}:step[${i}]: missing 'name' field`); + } + } + } +}); + +test('Workflows using github-script reference correct script paths', () => { + for (const file of workflowFiles) { + const content = fs.readFileSync(file, 'utf8'); + const workflow = yaml.load(content); + const workflowName = path.basename(file); + + for (const [jobName, job] of Object.entries(workflow.jobs)) { + for (const step of job.steps) { + if (step.uses && step.uses.includes('actions/github-script')) { + assert.ok(step.with && step.with.script, + `${workflowName}:${jobName}: github-script step must have 'with.script'`); + + // Check if script references a file + if (step.with.script.includes('require(')) { + const scriptPath = step.with.script.match(/require\('([^']+)'\)/)?.[1]; + if (scriptPath && scriptPath.startsWith('./.github/agent-workflow/scripts/')) { + const scriptFile = path.join(__dirname, '..', scriptPath); + assert.ok(fs.existsSync(scriptFile), + `${workflowName}:${jobName}: referenced script ${scriptPath} does not exist`); + } + } + } + } + } + } +});