Conversation
Automated sync from stranske/Workflows Template hash: a8050131ada4 Changes synced from sync-manifest.yml
🤖 Keepalive Loop StatusPR #844 | Agent: Codex | Iteration 0/5 Current State
🔍 Failure Classification| Error type | infrastructure | |
Keepalive Work Log (click to expand)
|
There was a problem hiding this comment.
Pull request overview
Syncs workflow templates to extend the weekly metrics aggregation pipeline with a new “bot-comment App auth coverage” preflight and artifact selection support, aligning this repo’s automation reporting with the upstream Workflows template set.
Changes:
- Add a bot-comment auth coverage preflight step to the weekly metrics workflow and include its outputs in uploaded artifacts.
- Extend weekly artifact selection to recognize new bot-comment auth coverage artifact families.
- Introduce a new
.github/scripts/bot_comment_auth_coverage.jsscript to scan downloaded artifacts and generate JSON + Markdown summaries.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/agents-weekly-metrics.yml |
Runs the new bot-comment auth coverage preflight and enforces coverage hard-block outcomes. |
.github/scripts/weekly_metrics_artifacts.js |
Adds bot-comment auth coverage artifact prefixes/families for bounded selection. |
.github/scripts/bot_comment_auth_coverage.js |
New preflight summarizer generating auth coverage JSON/Markdown from selected artifacts. |
| function summarizeOrganicEvidence(records = [], options = {}) { | ||
| const requiredEvents = parseCsvList( | ||
| options.required_organic_events ?? | ||
| options.requiredOrganicEvents ?? | ||
| process.env.BOT_COMMENT_AUTH_REQUIRED_ORGANIC_EVENTS | ||
| ); | ||
| const requiredComponents = parseCsvList( | ||
| options.organic_components ?? | ||
| options.organicComponents ?? | ||
| process.env.BOT_COMMENT_AUTH_ORGANIC_COMPONENTS | ||
| ); | ||
| 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 eventCounts = Object.create(null); | ||
| const latestByComponentEvent = Object.create(null); | ||
|
|
||
| for (const record of records) { | ||
| if (!record.component || !record.event_name) continue; | ||
| eventCounts[record.component] ||= {}; | ||
| eventCounts[record.component][record.event_name] = | ||
| (eventCounts[record.component][record.event_name] || 0) + 1; | ||
| const key = `${record.component}:${record.event_name}`; | ||
| const existing = latestByComponentEvent[key]; | ||
| if (!existing || compareRecords(record, existing) < 0) { | ||
| latestByComponentEvent[key] = record; | ||
| } | ||
| } | ||
|
|
||
| const blockers = []; | ||
| for (const component of components) { | ||
| for (const eventName of requiredEvents) { | ||
| const latest = latestByComponentEvent[`${component}:${eventName}`]; | ||
| if (!latest) { | ||
| 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 (expectedMode !== 'unknown' && latest.auth_mode !== expectedMode) { | ||
| blockers.push(`expected-${expectedMode}-organic-${component}-${eventName}`); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
summarizeOrganicEvidence() generates a missing-organic-* blocker for every required component/event pair even when there are zero auth records. Since summarizeBotCommentAuthCoverage() can still classify that scenario as coverage_status='no-data', the markdown ends up showing a long blocker list alongside a no-data status, which is confusing. Consider short-circuiting organic evidence to status 'no-data' (and no per-event blockers) when auth_record_count is 0, or only emitting missing-organic blockers once there is at least some organic evidence to evaluate.
| 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 | ||
| exit 1 |
There was a problem hiding this comment.
The final "Honor coverage hard-blocks" step exits with a generic 1 when either preflight fails, which loses the original exit code and doesn't indicate which check triggered the failure. Consider printing the two exit statuses (or a short message) before exiting, and/or exiting with the failing check's status to make debugging easier in job logs.
| exit 1 | |
| echo "Coverage hard-block triggered." | |
| if [ "${terminal_status}" != "0" ]; then | |
| echo "Review-thread terminal coverage preflight failed with exit status ${terminal_status}." | |
| fi | |
| if [ "${bot_comment_auth_status}" != "0" ]; then | |
| echo "Bot-comment auth coverage preflight failed with exit status ${bot_comment_auth_status}." | |
| fi | |
| if [ "${terminal_status}" != "0" ]; then | |
| exit "${terminal_status}" | |
| fi | |
| exit "${bot_comment_auth_status}" |
| for (const file of files) { | ||
| try { | ||
| const parsed = JSON.parse(fs.readFileSync(file, 'utf8')); | ||
| if (isAuthCoverageRecord(parsed)) { | ||
| records.push({ ...parsed, source_path: file }); | ||
| } | ||
| } catch (_error) { | ||
| parseErrors += 1; | ||
| } | ||
| } | ||
| return { records, parse_errors: parseErrors, file_count: files.length }; |
There was a problem hiding this comment.
readJsonRecords() silently ignores JSON files that parse successfully but don't match AUTH_SCHEMA. That makes it hard to distinguish a true "no data" week from a schema/producer mismatch, and it can cause the report to show parse_errors=0 and input_file_count>0 while auth_record_count=0. Consider tracking a separate non_auth_record_count (or treating unexpected-schema JSON as a parse/validation error) and using it to set coverage_status to warning, similar to how terminal_disposition_coverage.js reports non-terminal records.
| for (const file of files) { | |
| try { | |
| const parsed = JSON.parse(fs.readFileSync(file, 'utf8')); | |
| if (isAuthCoverageRecord(parsed)) { | |
| records.push({ ...parsed, source_path: file }); | |
| } | |
| } catch (_error) { | |
| parseErrors += 1; | |
| } | |
| } | |
| return { records, parse_errors: parseErrors, file_count: files.length }; | |
| let nonAuthRecordCount = 0; | |
| for (const file of files) { | |
| try { | |
| const parsed = JSON.parse(fs.readFileSync(file, 'utf8')); | |
| if (isAuthCoverageRecord(parsed)) { | |
| records.push({ ...parsed, source_path: file }); | |
| } else { | |
| nonAuthRecordCount += 1; | |
| parseErrors += 1; | |
| } | |
| } catch (_error) { | |
| parseErrors += 1; | |
| } | |
| } | |
| return { | |
| records, | |
| parse_errors: parseErrors, | |
| non_auth_record_count: nonAuthRecordCount, | |
| file_count: files.length, | |
| }; |
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Manifest:
.github/sync-manifest.yml