diff --git a/.github/scripts/bot_comment_auth_coverage.js b/.github/scripts/bot_comment_auth_coverage.js index 4f88bc80..cccddef4 100644 --- a/.github/scripts/bot_comment_auth_coverage.js +++ b/.github/scripts/bot_comment_auth_coverage.js @@ -9,6 +9,10 @@ const AUTH_ARTIFACT_FAMILIES = new Set([ 'bot-comment-auth-coverage-wrapper', 'bot-comment-auth-coverage-reusable', ]); +const AUTH_ARTIFACT_DIR_PATTERNS = { + wrapper: /^bot-comment-auth-coverage-wrapper-\d+$/, + reusable: /^bot-comment-auth-coverage-reusable-\d+$/, +}; const COMPONENT_POLICIES = { 'agents-bot-comment-handler-wrapper': { @@ -23,6 +27,21 @@ const COMPONENT_POLICIES = { }, }; +const COMPONENT_POLICY_OVERRIDES = { + 'agents-bot-comment-handler-wrapper': { + expected_option: 'wrapper_expected_mode', + expected_env: 'BOT_COMMENT_WRAPPER_EXPECTED_AUTH_MODE', + allowed_option: 'wrapper_allowed_modes', + allowed_env: 'BOT_COMMENT_WRAPPER_ALLOWED_AUTH_MODES', + }, + 'reusable-bot-comment-handler': { + expected_option: 'reusable_expected_mode', + expected_env: 'BOT_COMMENT_REUSABLE_EXPECTED_AUTH_MODE', + allowed_option: 'reusable_allowed_modes', + allowed_env: 'BOT_COMMENT_REUSABLE_ALLOWED_AUTH_MODES', + }, +}; + function cleanString(value) { if (value === null || value === undefined) return ''; return String(value).trim(); @@ -39,6 +58,7 @@ function normalizeRecordBoolean(value) { const text = cleanString(value).toLowerCase(); if (['1', 'true', 'yes', 'y', 'on'].includes(text)) return true; if (['0', 'false', 'no', 'n', 'off', ''].includes(text)) return false; + if (typeof value === 'string') return false; return Boolean(value); } @@ -108,7 +128,7 @@ function normalizeRecord(raw = {}, sourcePath = '') { } function isAuthCoverageRecord(record) { - return Boolean(record && typeof record === 'object' && record.schema === AUTH_SCHEMA); + return Boolean(record && typeof record === 'object' && cleanString(record.schema) === AUTH_SCHEMA); } function parseAllowedModes(value, fallback) { @@ -141,25 +161,43 @@ function parseCsvList(value) { .filter(Boolean); } +function firstConfiguredValue(options, keys, envName) { + for (const key of keys) { + if (Object.prototype.hasOwnProperty.call(options, key)) { + return { value: options[key], configured: true }; + } + } + if (Object.prototype.hasOwnProperty.call(process.env, envName)) { + return { value: process.env[envName], configured: true }; + } + return { value: undefined, configured: false }; +} + function summarizeOrganicEvidence(records = [], options = {}) { - const requiredEvents = parseCsvList( - options.required_organic_events ?? - options.requiredOrganicEvents ?? - process.env.BOT_COMMENT_AUTH_REQUIRED_ORGANIC_EVENTS + const requiredEventsConfig = firstConfiguredValue( + options, + ['required_organic_events', 'requiredOrganicEvents'], + 'BOT_COMMENT_AUTH_REQUIRED_ORGANIC_EVENTS' ); - const requiredComponents = parseCsvList( - options.organic_components ?? - options.organicComponents ?? - process.env.BOT_COMMENT_AUTH_ORGANIC_COMPONENTS + const organicComponentsConfig = firstConfiguredValue( + options, + ['organic_components', 'organicComponents'], + 'BOT_COMMENT_AUTH_ORGANIC_COMPONENTS' ); + const requiredEvents = parseCsvList(requiredEventsConfig.value); + const requiredComponents = parseCsvList(organicComponentsConfig.value); const expectedMode = normalizeAuthMode( options.organic_expected_mode ?? options.organicExpectedMode ?? process.env.BOT_COMMENT_AUTH_ORGANIC_EXPECTED_MODE ); - const components = requiredComponents.length > 0 - ? requiredComponents - : Object.keys(COMPONENT_POLICIES); + const organicChecksDisabled = requiredEvents.length === 0 || + (organicComponentsConfig.configured && requiredComponents.length === 0); + const components = organicChecksDisabled + ? [] + : requiredComponents.length > 0 + ? requiredComponents + : Object.keys(COMPONENT_POLICIES); const eventCounts = Object.create(null); const latestByComponentEvent = Object.create(null); @@ -171,13 +209,13 @@ function summarizeOrganicEvidence(records = [], options = {}) { expected_mode: expectedMode === 'unknown' ? '' : expectedMode, event_counts: eventCounts, blockers: [], - status: 'no-data', + status: organicChecksDisabled ? 'pass' : 'no-data', }; } for (const record of records) { if (!record.component || !record.event_name) continue; - eventCounts[record.component] ||= {}; + eventCounts[record.component] = eventCounts[record.component] || {}; eventCounts[record.component][record.event_name] = (eventCounts[record.component][record.event_name] || 0) + 1; const key = `${record.component}:${record.event_name}`; @@ -195,8 +233,11 @@ function summarizeOrganicEvidence(records = [], options = {}) { blockers.push(`missing-organic-${component}-${eventName}`); continue; } - if (latest.fallback_warning_active || latest.auth_mode === 'legacy-app-id') { - blockers.push(`legacy-organic-${component}-${eventName}`); + if (latest.fallback_warning_active) { + blockers.push(`legacy-organic-${component}-${eventName}-fallback-active`); + } + if (latest.auth_mode === 'legacy-app-id') { + blockers.push(`legacy-organic-${component}-${eventName}-auth-mode`); } if (expectedMode !== 'unknown' && latest.auth_mode !== expectedMode) { blockers.push(`expected-${expectedMode}-organic-${component}-${eventName}`); @@ -221,35 +262,20 @@ function componentPolicy(component, options = {}) { allowed_modes: ['client-id', 'none'], missing_record_severity: 'no-data', }; - if (component === 'agents-bot-comment-handler-wrapper') { - const expectedMode = parseExpectedMode( - options.wrapper_expected_mode ?? process.env.BOT_COMMENT_WRAPPER_EXPECTED_AUTH_MODE, - base.expected_mode - ); - return { - ...base, - ...expectedMode, - allowed_modes: parseAllowedModes( - options.wrapper_allowed_modes ?? process.env.BOT_COMMENT_WRAPPER_ALLOWED_AUTH_MODES, - base.allowed_modes - ), - }; - } - if (component === 'reusable-bot-comment-handler') { - const expectedMode = parseExpectedMode( - options.reusable_expected_mode ?? process.env.BOT_COMMENT_REUSABLE_EXPECTED_AUTH_MODE, - base.expected_mode - ); - return { - ...base, - ...expectedMode, - allowed_modes: parseAllowedModes( - options.reusable_allowed_modes ?? process.env.BOT_COMMENT_REUSABLE_ALLOWED_AUTH_MODES, - base.allowed_modes - ), - }; - } - return base; + const override = COMPONENT_POLICY_OVERRIDES[component]; + if (!override) return base; + const expectedMode = parseExpectedMode( + options[override.expected_option] ?? process.env[override.expected_env], + base.expected_mode + ); + return { + ...base, + ...expectedMode, + allowed_modes: parseAllowedModes( + options[override.allowed_option] ?? process.env[override.allowed_env], + base.allowed_modes + ), + }; } function runSortKey(record) { @@ -272,20 +298,20 @@ function compareRecords(a, b) { function normalizeArtifactSelectionSummary(report) { if (!report) return null; - if (report.status === 'missing' || report.status === 'parse-error') { + if (typeof report !== 'object' || Array.isArray(report)) { return { - schema: cleanString(report.schema) || 'workflows-weekly-metrics-artifact-selection/v1', - status: report.status, - error_message: cleanString(report.error_message), + schema: 'workflows-weekly-metrics-artifact-selection/v1', + status: 'parse-error', + error_message: 'artifact selection report is not a JSON object', selected_auth_artifact_count: 0, selected_auth_artifacts: [], }; } - if (typeof report !== 'object' || Array.isArray(report)) { + if (report.status === 'missing' || report.status === 'parse-error') { return { - schema: 'workflows-weekly-metrics-artifact-selection/v1', - status: 'parse-error', - error_message: 'artifact selection report is not a JSON object', + schema: cleanString(report.schema) || 'workflows-weekly-metrics-artifact-selection/v1', + status: report.status, + error_message: cleanString(report.error_message), selected_auth_artifact_count: 0, selected_auth_artifacts: [], }; @@ -324,7 +350,10 @@ function artifactFamilyFromSelection(artifact = {}) { function componentCoverageStatus(blockers, policy, latest) { if (blockers.length === 0) return 'pass'; - if (!latest && policy.missing_record_severity === 'no-data') return 'no-data'; + const onlyMissingBlockers = blockers.every((blocker) => isComponentMissingBlocker(blocker)); + if (!latest && policy.missing_record_severity === 'no-data' && onlyMissingBlockers) { + return 'no-data'; + } return 'warning'; } @@ -335,6 +364,15 @@ function isComponentMissingBlocker(blocker) { function summarizeBotCommentAuthCoverage(records = [], options = {}) { const policy = normalizePolicy(options); const parseErrors = Number(options.parse_errors ?? options.parseErrors ?? 0); + const readErrors = Number(options.read_errors ?? options.readErrors ?? 0); + const parsedJsonRecordCount = Number( + options.parsed_json_record_count ?? options.parsedJsonRecordCount ?? records.length + ); + const nonAuthRecordCount = Number( + options.non_auth_record_count ?? + options.nonAuthRecordCount ?? + Math.max(0, parsedJsonRecordCount - records.length) + ); const artifactSelection = normalizeArtifactSelectionSummary( options.artifact_selection_report ?? options.artifactSelectionReport ); @@ -369,9 +407,12 @@ function summarizeBotCommentAuthCoverage(records = [], options = {}) { if (!componentPolicyConfig.allowed_modes.includes(latest.auth_mode)) { blockers.push(`disallowed-${component}-auth-mode`); } - if (latest.fallback_warning_active || latest.auth_mode === 'legacy-app-id') { + if (latest.fallback_warning_active) { blockers.push(`legacy-${component}-fallback-active`); } + if (latest.auth_mode === 'legacy-app-id') { + blockers.push(`legacy-${component}-auth-mode`); + } if ( componentPolicyConfig.expected_mode && latest.auth_mode !== componentPolicyConfig.expected_mode @@ -386,6 +427,7 @@ function summarizeBotCommentAuthCoverage(records = [], options = {}) { expected_mode: componentPolicyConfig.expected_mode, invalid_expected_mode: componentPolicyConfig.invalid_expected_mode, allowed_modes: componentPolicyConfig.allowed_modes, + missing_record_severity: componentPolicyConfig.missing_record_severity, status: componentCoverageStatus(blockers, componentPolicyConfig, latest), blockers, }; @@ -396,8 +438,15 @@ function summarizeBotCommentAuthCoverage(records = [], options = {}) { artifactSelection.status !== 'not-configured'; const selectedAuthArtifactCount = artifactSelection?.selected_auth_artifact_count || 0; const authArtifactInputMismatch = selectedAuthArtifactCount > 0 && inputFileCount === 0; - const blockers = componentSummaries.flatMap((summary) => summary.blockers); + const blockers = componentSummaries.flatMap((summary) => + summary.blockers.filter( + (blocker) => + !(summary.missing_record_severity === 'no-data' && isComponentMissingBlocker(blocker)) + ) + ); if (parseErrors > 0) blockers.push('parse-errors'); + if (readErrors > 0) blockers.push('read-errors'); + if (nonAuthRecordCount > 0) blockers.push('non-auth-records'); if (artifactSelectionWarning) blockers.push('artifact-selection-warning'); if (authArtifactInputMismatch) blockers.push('selected-auth-artifacts-without-input-files'); blockers.push(...organicEvidence.blockers); @@ -411,7 +460,7 @@ function summarizeBotCommentAuthCoverage(records = [], options = {}) { } const hardBlockActive = policy.effective_mode === HARD_BLOCK_MODE; - const shouldFail = hardBlockActive && coverageStatus !== 'pass'; + const shouldFail = hardBlockActive && coverageStatus === 'warning'; return { schema: COVERAGE_SCHEMA, status: shouldFail ? 'fail' : coverageStatus, @@ -430,9 +479,11 @@ function summarizeBotCommentAuthCoverage(records = [], options = {}) { }, input_file_count: inputFileCount, input_files: inputFiles, - scanned_record_count: records.length, + scanned_record_count: parsedJsonRecordCount, auth_record_count: authRecords.length, + non_auth_record_count: nonAuthRecordCount, parse_errors: parseErrors, + read_errors: readErrors, auth_artifact_input_mismatch: authArtifactInputMismatch, artifact_selection: artifactSelection, organic_evidence: organicEvidence, @@ -450,11 +501,15 @@ function formatBotCommentAuthCoverageMarkdown(report) { `- Mode: ${report.mode}`, `- Hard block active: ${report.enforcement.hard_block_active}`, `- Input files: ${report.input_file_count}`, + `- Scanned JSON records: ${report.scanned_record_count}`, `- Auth records: ${report.auth_record_count}`, + `- Non-auth records: ${report.non_auth_record_count}`, `- Parse errors: ${report.parse_errors}`, + `- Read errors: ${report.read_errors}`, ]; if (report.artifact_selection) { + lines.push(`- Artifact selection status: ${report.artifact_selection.status || 'unknown'}`); lines.push(`- Selected auth artifacts: ${report.artifact_selection.selected_auth_artifact_count}`); if (report.artifact_selection.error_message) { lines.push(`- Artifact selector error: ${report.artifact_selection.error_message}`); @@ -493,9 +548,21 @@ function collectJsonFiles(rootDir) { const stack = [rootDir]; while (stack.length > 0) { const current = stack.pop(); - const stat = fs.statSync(current); + if (!current || !fs.existsSync(current)) continue; + let stat; + try { + stat = fs.statSync(current); + } catch (_error) { + continue; + } if (stat.isDirectory()) { - for (const entry of fs.readdirSync(current)) { + let entries = []; + try { + entries = fs.readdirSync(current); + } catch (_error) { + continue; + } + for (const entry of entries) { stack.push(path.join(current, entry)); } } else if (stat.isFile() && isPotentialAuthCoverageFile(current)) { @@ -509,31 +576,47 @@ function isPotentialAuthCoverageFile(file) { const normalized = cleanString(file).split(path.sep).join('/'); const basename = path.basename(normalized); if (!normalized.endsWith('.json')) return false; - const segments = normalized.split('/'); - const hasWrapperArtifactDir = segments.some((segment) => - segment.startsWith('bot-comment-auth-coverage-wrapper-') + const artifactDir = path.basename(path.dirname(normalized)); + return ( + (basename === 'wrapper.json' && AUTH_ARTIFACT_DIR_PATTERNS.wrapper.test(artifactDir)) || + (basename === 'reusable.json' && AUTH_ARTIFACT_DIR_PATTERNS.reusable.test(artifactDir)) ); - const hasReusableArtifactDir = segments.some((segment) => - segment.startsWith('bot-comment-auth-coverage-reusable-') - ); - return (basename === 'wrapper.json' && hasWrapperArtifactDir) || - (basename === 'reusable.json' && hasReusableArtifactDir); } function readJsonRecords(files = []) { const records = []; let parseErrors = 0; + let readErrors = 0; + let parsedJsonRecordCount = 0; + let nonAuthRecordCount = 0; for (const file of files) { + let content = ''; + try { + content = fs.readFileSync(file, 'utf8'); + } catch (_error) { + readErrors += 1; + continue; + } try { - const parsed = JSON.parse(fs.readFileSync(file, 'utf8')); + const parsed = JSON.parse(content); + parsedJsonRecordCount += 1; if (isAuthCoverageRecord(parsed)) { records.push({ ...parsed, source_path: file }); + } else { + nonAuthRecordCount += 1; } } catch (_error) { parseErrors += 1; } } - return { records, parse_errors: parseErrors, file_count: files.length }; + return { + records, + parse_errors: parseErrors, + read_errors: readErrors, + parsed_json_record_count: parsedJsonRecordCount, + non_auth_record_count: nonAuthRecordCount, + file_count: files.length, + }; } function readArtifactSelectionReport(file) { @@ -598,6 +681,9 @@ function main() { const readResult = readJsonRecords(files); const report = summarizeBotCommentAuthCoverage(readResult.records, { parse_errors: readResult.parse_errors, + read_errors: readResult.read_errors, + parsed_json_record_count: readResult.parsed_json_record_count, + non_auth_record_count: readResult.non_auth_record_count, input_files: files, input_file_count: readResult.file_count, artifact_selection_report: readArtifactSelectionReport(options.artifact_selection_report), diff --git a/.github/workflows/agents-weekly-metrics.yml b/.github/workflows/agents-weekly-metrics.yml index c73ec3e4..ad4ea625 100644 --- a/.github/workflows/agents-weekly-metrics.yml +++ b/.github/workflows/agents-weekly-metrics.yml @@ -42,8 +42,10 @@ jobs: scripts/aggregate_agent_metrics.py sparse-checkout-cone-mode: false - - name: Install GitHub API dependencies - run: npm install --no-save --no-package-lock @octokit/rest @octokit/auth-app + - name: Setup Node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6 + with: + node-version: "20" - name: Setup API client uses: ./.github/actions/setup-api-client @@ -290,9 +292,10 @@ jobs: terminal_status="${TERMINAL_DISPOSITION_COVERAGE_EXIT_STATUS:-0}" bot_comment_auth_status="${BOT_COMMENT_AUTH_COVERAGE_EXIT_STATUS:-0}" if [ "${terminal_status}" != "0" ] || [ "${bot_comment_auth_status}" != "0" ]; then - echo "::error::Coverage hard-block triggered: " \ - "terminal-disposition=${terminal_status}, " \ - "bot-comment-auth=${bot_comment_auth_status}" + echo "::error title=Coverage hard-block failed::" \ + "terminal-disposition-exit-status=${terminal_status}; " \ + "bot-comment-auth-exit-status=${bot_comment_auth_status}; " \ + "coverage reports were uploaded before this failure" if [ "${terminal_status}" != "0" ]; then echo "Failed check: Review-thread terminal coverage preflight" fi