diff --git a/.github/scripts/__tests__/bot-comment-auth-coverage.test.js b/.github/scripts/__tests__/bot-comment-auth-coverage.test.js index 6571efdc4..feb9942ab 100644 --- a/.github/scripts/__tests__/bot-comment-auth-coverage.test.js +++ b/.github/scripts/__tests__/bot-comment-auth-coverage.test.js @@ -656,6 +656,73 @@ test('warns when required organic bot auth evidence is missing', () => { ); }); +test('skips reusable organic requirement when the wrapper intentionally skipped it', () => { + const records = [ + record('agents-bot-comment-handler-wrapper', 'client-id', 501, { + event_name: 'pull_request', + reusable_invocation_expected: false, + reusable_invocation_reason: 'wrapper-skipped', + }), + record('agents-bot-comment-handler-wrapper', 'client-id', 502, { + event_name: 'workflow_run', + reusable_invocation_expected: 'false', + reusable_invocation_reason: 'wrapper-skipped', + }), + ]; + const report = summarizeBotCommentAuthCoverage(records, { + required_organic_events: 'pull_request,workflow_run', + organic_components: 'agents-bot-comment-handler-wrapper,reusable-bot-comment-handler', + organic_expected_mode: 'client-id', + reusable_expected_mode: '', + }); + const markdown = formatBotCommentAuthCoverageMarkdown(report); + + assert.equal(report.organic_evidence.status, 'pass'); + assert.deepEqual(report.organic_evidence.blockers, []); + assert.deepEqual( + report.organic_evidence.skipped_requirements.map((item) => `${item.component}:${item.event_name}`), + [ + 'reusable-bot-comment-handler:pull_request', + 'reusable-bot-comment-handler:workflow_run', + ] + ); + assert.equal( + report.enforcement.blockers.some((blocker) => + blocker.startsWith('missing-organic-reusable-bot-comment-handler') + ), + false + ); + assert.match( + markdown, + /Skipped organic requirements: reusable-bot-comment-handler\/pull_request, reusable-bot-comment-handler\/workflow_run/ + ); +}); + +test('requires reusable organic evidence when the wrapper expected to call it', () => { + const report = summarizeBotCommentAuthCoverage( + [ + record('agents-bot-comment-handler-wrapper', 'client-id', 503, { + event_name: 'pull_request', + reusable_invocation_expected: true, + }), + ], + { + required_organic_events: 'pull_request', + organic_components: 'agents-bot-comment-handler-wrapper,reusable-bot-comment-handler', + organic_expected_mode: 'client-id', + reusable_expected_mode: '', + } + ); + + assert.equal(report.organic_evidence.status, 'warning'); + assert.ok( + report.enforcement.blockers.includes( + 'missing-organic-reusable-bot-comment-handler-pull_request' + ) + ); + assert.deepEqual(report.organic_evidence.skipped_requirements, []); +}); + test('keeps explicit zero parse errors and reports missing organic evidence as no-data without auth records', () => { const organic = summarizeOrganicEvidence([], { required_organic_events: 'pull_request', diff --git a/.github/scripts/__tests__/weekly-metrics-artifacts.test.js b/.github/scripts/__tests__/weekly-metrics-artifacts.test.js index 3ba7d31cc..727a57b34 100644 --- a/.github/scripts/__tests__/weekly-metrics-artifacts.test.js +++ b/.github/scripts/__tests__/weekly-metrics-artifacts.test.js @@ -8,6 +8,7 @@ const { collectRepoArtifacts, formatArtifactTsv, formatSelectionMarkdown, + missingPriorityFamilies, normalizeSelectionOptions, selectMetricsArtifacts, } = require('../weekly_metrics_artifacts.js'); @@ -96,6 +97,10 @@ test('selects only recent matching artifacts with a machine-readable report', () 'keepalive-metrics': 1, 'review-thread-terminal-disposition': 1, }); + assert.deepEqual(report.missing_priority_families, [ + 'verifier-terminal-disposition', + 'bot-comment-auth-coverage-reusable', + ]); assert.deepEqual(report.selected_family_counts, { 'autopilot-metrics': 1, 'bot-comment-auth-coverage-wrapper': 2, @@ -119,6 +124,12 @@ test('builds an error report when artifact selection cannot query GitHub', () => assert.equal(report.status, 'error'); assert.equal(report.error_message, 'API rate limit exceeded'); assert.equal(report.selected_count, 0); + assert.deepEqual(report.missing_priority_families, [ + 'verifier-terminal-disposition', + 'review-thread-terminal-disposition', + 'bot-comment-auth-coverage-wrapper', + 'bot-comment-auth-coverage-reusable', + ]); assert.deepEqual(report.selected_artifacts, []); assert.match(markdown, /Status: error/); assert.match(markdown, /API rate limit exceeded/); @@ -182,6 +193,20 @@ test('reserves priority telemetry artifacts before filling the total cap', () => 'review-thread-terminal-disposition': 1, 'verifier-terminal-disposition': 1, }); + assert.deepEqual(report.missing_priority_families, []); +}); + +test('reports priority telemetry families that are absent from the scan', () => { + const counts = new Map([ + ['bot-comment-auth-coverage-wrapper', 2], + ['keepalive-metrics', 5], + ]); + + assert.deepEqual(missingPriorityFamilies(counts), [ + 'verifier-terminal-disposition', + 'review-thread-terminal-disposition', + 'bot-comment-auth-coverage-reusable', + ]); }); test('formats selected artifacts for the workflow download loop', () => { @@ -207,6 +232,10 @@ test('formats a human-visible selector summary for weekly metrics', () => { assert.match(markdown, /Weekly Metrics Artifact Selection/); assert.match(markdown, /Scan cap: 5 pages x 100 artifacts/); assert.match(markdown, /Selected artifacts: 2/); + assert.match( + markdown, + /Missing priority families: verifier-terminal-disposition, review-thread-terminal-disposition, bot-comment-auth-coverage-wrapper, bot-comment-auth-coverage-reusable/ + ); assert.match(markdown, /Artifact family \| Candidates \| Selected/); assert.match(markdown, /autopilot-metrics/); assert.match(markdown, /keepalive-metrics/); diff --git a/.github/scripts/bot_comment_auth_coverage.js b/.github/scripts/bot_comment_auth_coverage.js index 0ac9369cc..443230f49 100644 --- a/.github/scripts/bot_comment_auth_coverage.js +++ b/.github/scripts/bot_comment_auth_coverage.js @@ -73,6 +73,11 @@ function normalizeRecordBoolean(value) { return Boolean(value); } +function normalizeOptionalRecordBoolean(value) { + if (value === null || value === undefined || cleanString(value) === '') return null; + return normalizeRecordBoolean(value); +} + function normalizeMode(value) { const text = cleanString(value).toLowerCase(); if (['hard-block', 'hard_block', 'hard', 'block', 'blocking', 'enforce'].includes(text)) { @@ -134,6 +139,12 @@ function normalizeRecord(raw = {}, sourcePath = '') { fallback_warning_active: normalizeRecordBoolean( raw.fallback_warning_active ?? raw.fallbackWarningActive ), + reusable_invocation_expected: normalizeOptionalRecordBoolean( + raw.reusable_invocation_expected ?? raw.reusableInvocationExpected + ), + reusable_invocation_reason: cleanString( + raw.reusable_invocation_reason ?? raw.reusableInvocationReason + ), source_path: sourcePath, }; } @@ -222,6 +233,7 @@ function summarizeOrganicEvidence(records = [], options = {}) { required_components: components, expected_mode: expectedMode === 'unknown' ? '' : expectedMode, event_counts: eventCounts, + skipped_requirements: [], blockers, status: organicChecksDisabled ? 'pass' : 'no-data', }; @@ -240,9 +252,25 @@ function summarizeOrganicEvidence(records = [], options = {}) { } const blockers = []; + const skippedRequirements = []; for (const component of components) { for (const eventName of requiredEvents) { const latest = latestByComponentEvent[`${component}:${eventName}`]; + const latestWrapper = latestByComponentEvent[ + `agents-bot-comment-handler-wrapper:${eventName}` + ]; + const reusableWasNotExpected = component === 'reusable-bot-comment-handler' && + latestWrapper && + latestWrapper.reusable_invocation_expected === false; + if (!latest && reusableWasNotExpected) { + skippedRequirements.push({ + component, + event_name: eventName, + reason: latestWrapper.reusable_invocation_reason || 'wrapper-did-not-call-reusable', + wrapper_run_id: latestWrapper.run_id, + }); + continue; + } if (!latest) { blockers.push(`missing-organic-${component}-${eventName}`); continue; @@ -265,6 +293,7 @@ function summarizeOrganicEvidence(records = [], options = {}) { required_components: components, expected_mode: expectedMode === 'unknown' ? '' : expectedMode, event_counts: eventCounts, + skipped_requirements: skippedRequirements, blockers, status: blockers.length > 0 ? 'warning' : 'pass', }; @@ -555,6 +584,14 @@ function formatBotCommentAuthCoverageMarkdown(report) { if (report.organic_evidence?.required_events?.length > 0) { lines.push(`- Required organic events: ${report.organic_evidence.required_events.join(', ')}`); lines.push(`- Organic evidence status: ${report.organic_evidence.status}`); + const skipped = report.organic_evidence.skipped_requirements || []; + if (skipped.length > 0) { + lines.push( + `- Skipped organic requirements: ${skipped + .map((item) => `${item.component}/${item.event_name}`) + .join(', ')}` + ); + } } if (report.enforcement.blockers.length > 0) { lines.push(`- Blockers: ${report.enforcement.blockers.join(', ')}`); diff --git a/.github/scripts/weekly_metrics_artifacts.js b/.github/scripts/weekly_metrics_artifacts.js index a8f6cb86c..baa0fe3bf 100644 --- a/.github/scripts/weekly_metrics_artifacts.js +++ b/.github/scripts/weekly_metrics_artifacts.js @@ -128,6 +128,10 @@ function sortedCountObject(counts) { ); } +function missingPriorityFamilies(candidateFamilyCounts = new Map()) { + return PRIORITY_METRICS_FAMILIES.filter((family) => !candidateFamilyCounts.has(family)); +} + function selectMetricsArtifacts(artifacts = [], options = {}) { const config = normalizeSelectionOptions(options); const stats = { @@ -223,6 +227,7 @@ function selectMetricsArtifacts(artifacts = [], options = {}) { ...stats, candidate_family_counts: sortedCountObject(candidateFamilyCounts), selected_family_counts: sortedCountObject(familyCounts), + missing_priority_families: missingPriorityFamilies(candidateFamilyCounts), selected_artifacts: selected.map((artifact) => ({ id: artifact.id, name: artifact.name, @@ -257,6 +262,7 @@ function buildSelectionErrorReport(options = {}, error = {}) { ignored_total_limit_count: 0, candidate_family_counts: {}, selected_family_counts: {}, + missing_priority_families: [...PRIORITY_METRICS_FAMILIES], selected_artifacts: [], }; } @@ -283,6 +289,7 @@ function formatSelectionMarkdown(report) { `- Scanned artifacts: ${report.scanned_count}`, `- Candidate artifacts: ${report.candidate_count}`, `- Selected artifacts: ${report.selected_count}`, + `- Missing priority families: ${(report.missing_priority_families || []).join(', ') || 'none'}`, `- Ignored: ${report.ignored_old_count} old, ${report.ignored_expired_count} expired, ` + `${report.ignored_name_count} non-metrics, ${report.ignored_family_limit_count} over family cap, ` + `${report.ignored_total_limit_count} over total cap`, @@ -428,6 +435,7 @@ module.exports = { collectRepoArtifacts, formatArtifactTsv, formatSelectionMarkdown, + missingPriorityFamilies, normalizeSelectionOptions, selectMetricsArtifacts, }; diff --git a/.github/workflows/agents-bot-comment-handler.yml b/.github/workflows/agents-bot-comment-handler.yml index ea5df4c2a..08e4dd21e 100644 --- a/.github/workflows/agents-bot-comment-handler.yml +++ b/.github/workflows/agents-bot-comment-handler.yml @@ -99,48 +99,6 @@ jobs: echo "workflow_app_auth_mode=${workflow_app_auth_mode}" } >> "$GITHUB_OUTPUT" - - name: Write wrapper App auth coverage - if: always() - env: - WORKFLOW_APP_AUTH_MODE: ${{ steps.workflow-app-creds.outputs.workflow_app_auth_mode }} - WORKFLOW_APP_CLIENT_ID_CONFIGURED: ${{ steps.workflow-app-creds.outputs.client_id_configured }} - WORKFLOW_APP_LEGACY_APP_ID_CONFIGURED: ${{ steps.workflow-app-creds.outputs.legacy_app_id_configured }} - WORKFLOW_APP_PRIVATE_KEY_CONFIGURED: ${{ steps.workflow-app-creds.outputs.private_key_configured }} - run: | - mkdir -p bot-comment-auth-coverage - node <<'NODE' - const fs = require('fs'); - const truthy = (value) => String(value || '').toLowerCase() === 'true'; - const record = { - schema: 'workflows-bot-comment-auth-coverage/v1', - component: 'agents-bot-comment-handler-wrapper', - repository: process.env.GITHUB_REPOSITORY || '', - workflow: process.env.GITHUB_WORKFLOW || '', - run_id: process.env.GITHUB_RUN_ID || '', - run_attempt: process.env.GITHUB_RUN_ATTEMPT || '', - event_name: process.env.GITHUB_EVENT_NAME || '', - auth_mode: process.env.WORKFLOW_APP_AUTH_MODE || 'none', - client_id_configured: truthy(process.env.WORKFLOW_APP_CLIENT_ID_CONFIGURED), - legacy_app_id_configured: truthy(process.env.WORKFLOW_APP_LEGACY_APP_ID_CONFIGURED), - private_key_configured: truthy(process.env.WORKFLOW_APP_PRIVATE_KEY_CONFIGURED), - fallback_warning_active: process.env.WORKFLOW_APP_AUTH_MODE === 'legacy-app-id', - direct_jobs_covered: ['resolve', 'cleanup'], - }; - fs.writeFileSync( - 'bot-comment-auth-coverage/wrapper.json', - `${JSON.stringify(record, null, 2)}\n` - ); - NODE - - - name: Upload wrapper App auth coverage - if: always() - uses: actions/upload-artifact@v7 - with: - name: bot-comment-auth-coverage-wrapper-${{ github.run_id }} - path: bot-comment-auth-coverage/wrapper.json - if-no-files-found: error - retention-days: 14 - - name: Mint GitHub App Token (client ID) id: app_token_client uses: actions/create-github-app-token@v3 @@ -312,6 +270,56 @@ jobs: core.setOutput('pr_number', prNumber || ''); core.setOutput('should_run', shouldRun ? 'true' : 'false'); + - name: Write wrapper App auth coverage + if: always() + env: + WORKFLOW_APP_AUTH_MODE: ${{ steps.workflow-app-creds.outputs.workflow_app_auth_mode }} + WORKFLOW_APP_CLIENT_ID_CONFIGURED: ${{ steps.workflow-app-creds.outputs.client_id_configured }} + WORKFLOW_APP_LEGACY_APP_ID_CONFIGURED: ${{ steps.workflow-app-creds.outputs.legacy_app_id_configured }} + WORKFLOW_APP_PRIVATE_KEY_CONFIGURED: ${{ steps.workflow-app-creds.outputs.private_key_configured }} + RESOLVED_PR_NUMBER: ${{ steps.resolve.outputs.pr_number }} + REUSABLE_INVOCATION_EXPECTED: ${{ steps.resolve.outputs.should_run }} + run: | + mkdir -p bot-comment-auth-coverage + node <<'NODE' + const fs = require('fs'); + const truthy = (value) => String(value || '').toLowerCase() === 'true'; + const reusableExpected = truthy(process.env.REUSABLE_INVOCATION_EXPECTED); + const record = { + schema: 'workflows-bot-comment-auth-coverage/v1', + component: 'agents-bot-comment-handler-wrapper', + repository: process.env.GITHUB_REPOSITORY || '', + workflow: process.env.GITHUB_WORKFLOW || '', + run_id: process.env.GITHUB_RUN_ID || '', + run_attempt: process.env.GITHUB_RUN_ATTEMPT || '', + event_name: process.env.GITHUB_EVENT_NAME || '', + auth_mode: process.env.WORKFLOW_APP_AUTH_MODE || 'none', + client_id_configured: truthy(process.env.WORKFLOW_APP_CLIENT_ID_CONFIGURED), + legacy_app_id_configured: truthy(process.env.WORKFLOW_APP_LEGACY_APP_ID_CONFIGURED), + private_key_configured: truthy(process.env.WORKFLOW_APP_PRIVATE_KEY_CONFIGURED), + fallback_warning_active: process.env.WORKFLOW_APP_AUTH_MODE === 'legacy-app-id', + resolved_pr_number: process.env.RESOLVED_PR_NUMBER || '', + reusable_invocation_expected: reusableExpected, + reusable_invocation_reason: reusableExpected + ? 'wrapper-should-run' + : 'wrapper-skipped', + direct_jobs_covered: ['resolve', 'cleanup'], + }; + fs.writeFileSync( + 'bot-comment-auth-coverage/wrapper.json', + `${JSON.stringify(record, null, 2)}\n` + ); + NODE + + - name: Upload wrapper App auth coverage + if: always() + uses: actions/upload-artifact@v7 + with: + name: bot-comment-auth-coverage-wrapper-${{ github.run_id }} + path: bot-comment-auth-coverage/wrapper.json + if-no-files-found: error + retention-days: 14 + # Call the reusable workflow handle: name: Handle bot comments diff --git a/templates/consumer-repo/.github/scripts/bot_comment_auth_coverage.js b/templates/consumer-repo/.github/scripts/bot_comment_auth_coverage.js index 0ac9369cc..443230f49 100644 --- a/templates/consumer-repo/.github/scripts/bot_comment_auth_coverage.js +++ b/templates/consumer-repo/.github/scripts/bot_comment_auth_coverage.js @@ -73,6 +73,11 @@ function normalizeRecordBoolean(value) { return Boolean(value); } +function normalizeOptionalRecordBoolean(value) { + if (value === null || value === undefined || cleanString(value) === '') return null; + return normalizeRecordBoolean(value); +} + function normalizeMode(value) { const text = cleanString(value).toLowerCase(); if (['hard-block', 'hard_block', 'hard', 'block', 'blocking', 'enforce'].includes(text)) { @@ -134,6 +139,12 @@ function normalizeRecord(raw = {}, sourcePath = '') { fallback_warning_active: normalizeRecordBoolean( raw.fallback_warning_active ?? raw.fallbackWarningActive ), + reusable_invocation_expected: normalizeOptionalRecordBoolean( + raw.reusable_invocation_expected ?? raw.reusableInvocationExpected + ), + reusable_invocation_reason: cleanString( + raw.reusable_invocation_reason ?? raw.reusableInvocationReason + ), source_path: sourcePath, }; } @@ -222,6 +233,7 @@ function summarizeOrganicEvidence(records = [], options = {}) { required_components: components, expected_mode: expectedMode === 'unknown' ? '' : expectedMode, event_counts: eventCounts, + skipped_requirements: [], blockers, status: organicChecksDisabled ? 'pass' : 'no-data', }; @@ -240,9 +252,25 @@ function summarizeOrganicEvidence(records = [], options = {}) { } const blockers = []; + const skippedRequirements = []; for (const component of components) { for (const eventName of requiredEvents) { const latest = latestByComponentEvent[`${component}:${eventName}`]; + const latestWrapper = latestByComponentEvent[ + `agents-bot-comment-handler-wrapper:${eventName}` + ]; + const reusableWasNotExpected = component === 'reusable-bot-comment-handler' && + latestWrapper && + latestWrapper.reusable_invocation_expected === false; + if (!latest && reusableWasNotExpected) { + skippedRequirements.push({ + component, + event_name: eventName, + reason: latestWrapper.reusable_invocation_reason || 'wrapper-did-not-call-reusable', + wrapper_run_id: latestWrapper.run_id, + }); + continue; + } if (!latest) { blockers.push(`missing-organic-${component}-${eventName}`); continue; @@ -265,6 +293,7 @@ function summarizeOrganicEvidence(records = [], options = {}) { required_components: components, expected_mode: expectedMode === 'unknown' ? '' : expectedMode, event_counts: eventCounts, + skipped_requirements: skippedRequirements, blockers, status: blockers.length > 0 ? 'warning' : 'pass', }; @@ -555,6 +584,14 @@ function formatBotCommentAuthCoverageMarkdown(report) { if (report.organic_evidence?.required_events?.length > 0) { lines.push(`- Required organic events: ${report.organic_evidence.required_events.join(', ')}`); lines.push(`- Organic evidence status: ${report.organic_evidence.status}`); + const skipped = report.organic_evidence.skipped_requirements || []; + if (skipped.length > 0) { + lines.push( + `- Skipped organic requirements: ${skipped + .map((item) => `${item.component}/${item.event_name}`) + .join(', ')}` + ); + } } if (report.enforcement.blockers.length > 0) { lines.push(`- Blockers: ${report.enforcement.blockers.join(', ')}`); diff --git a/templates/consumer-repo/.github/scripts/weekly_metrics_artifacts.js b/templates/consumer-repo/.github/scripts/weekly_metrics_artifacts.js index a8f6cb86c..baa0fe3bf 100644 --- a/templates/consumer-repo/.github/scripts/weekly_metrics_artifacts.js +++ b/templates/consumer-repo/.github/scripts/weekly_metrics_artifacts.js @@ -128,6 +128,10 @@ function sortedCountObject(counts) { ); } +function missingPriorityFamilies(candidateFamilyCounts = new Map()) { + return PRIORITY_METRICS_FAMILIES.filter((family) => !candidateFamilyCounts.has(family)); +} + function selectMetricsArtifacts(artifacts = [], options = {}) { const config = normalizeSelectionOptions(options); const stats = { @@ -223,6 +227,7 @@ function selectMetricsArtifacts(artifacts = [], options = {}) { ...stats, candidate_family_counts: sortedCountObject(candidateFamilyCounts), selected_family_counts: sortedCountObject(familyCounts), + missing_priority_families: missingPriorityFamilies(candidateFamilyCounts), selected_artifacts: selected.map((artifact) => ({ id: artifact.id, name: artifact.name, @@ -257,6 +262,7 @@ function buildSelectionErrorReport(options = {}, error = {}) { ignored_total_limit_count: 0, candidate_family_counts: {}, selected_family_counts: {}, + missing_priority_families: [...PRIORITY_METRICS_FAMILIES], selected_artifacts: [], }; } @@ -283,6 +289,7 @@ function formatSelectionMarkdown(report) { `- Scanned artifacts: ${report.scanned_count}`, `- Candidate artifacts: ${report.candidate_count}`, `- Selected artifacts: ${report.selected_count}`, + `- Missing priority families: ${(report.missing_priority_families || []).join(', ') || 'none'}`, `- Ignored: ${report.ignored_old_count} old, ${report.ignored_expired_count} expired, ` + `${report.ignored_name_count} non-metrics, ${report.ignored_family_limit_count} over family cap, ` + `${report.ignored_total_limit_count} over total cap`, @@ -428,6 +435,7 @@ module.exports = { collectRepoArtifacts, formatArtifactTsv, formatSelectionMarkdown, + missingPriorityFamilies, normalizeSelectionOptions, selectMetricsArtifacts, };