chore: sync workflow templates - #5190
Conversation
Automated sync from stranske/Workflows Template hash: 20d0e241bca2 Changes synced from sync-manifest.yml
stranske
added
sync
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 760f4fb19b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| families.includes(artifactFamily(artifact.name)) | ||
| ); | ||
| artifacts.push(...matchingArtifacts); | ||
| if (familiesSatisfied(artifacts, families, config)) { |
There was a problem hiding this comment.
collectPriorityWorkflowArtifacts uses the shared artifacts array when calling familiesSatisfied, so later workflow sources that share a family with earlier sources can stop after the first run even if that run produced no matching artifact. In practice, if source A already yielded verifier-terminal-disposition, source B will break immediately after inspecting its newest run, skipping older runs that may contain the only usable artifact for that source; this can silently select stale or missing weekly metrics artifacts.
Useful? React with 👍 / 👎.
| /\b(?:close[sd]?|closing|fix(?:e[sd])?|fixing|resolve[sd]?|resolving|address(?:e[sd])?|addressing)\s*[:#-]?\s*#([0-9]+)\b/gi, | ||
| /\b(?:(?:relate[sd]?\s+to|refs?|references?)\s+(?:issue\s+)?|(?:source|github|linked)\s+issue\s*)[:#-]?\s*#([0-9]+)\b/gi, | ||
| ]; |
There was a problem hiding this comment.
resolveNonIssueWorkflowSourceContextForBodySync relies on extractExplicitIssueSyncNumbers, but the regex list only matches textual references like Closes #123 and omits <!-- meta:issue:123 -->. When a PR has both an explicit non-issue workflow marker and only a meta issue marker, the function treats it as non-issue and skips issue-backed body sync even though the issue link is explicit, which can leave PR metadata unsynced from its source issue.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Syncs workflow-consumer helper scripts from stranske/Workflows into this repo, updating metrics aggregation/selection and PR workflow-source handling used by the weekly metrics and PR metadata automations.
Changes:
- Adds “priority producer” artifact collection and deduping to improve weekly metrics artifact selection.
- Expands workflow source parsing to detect “no automation” intent and threads that signal into keepalive/PR metadata flows.
- Hardens a few contracts and filesystem operations (typed
Counters in Python aggregation; saferstatSynchandling).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/aggregate_agent_metrics.py |
Adds typed Counter[...] annotations and makes missing_priority_families contract parsing more defensive. |
.github/scripts/weekly_metrics_artifacts.js |
Introduces priority-workflow artifact scanning, deduping, and a new selection option to cap runs per source. |
.github/scripts/source_context.js |
Adds “no automation” detection from labels/markers/templates; refactors template parsing helpers. |
.github/scripts/coverage_monitor_summary.js |
Wraps statSync in try/catch to avoid race/permission failures. |
.github/scripts/agents_pr_meta_update_body.js |
Refines “explicit non-issue context” logic and resolves workflow-source repair comments more consistently. |
.github/scripts/agents_pr_meta_keepalive.js |
Skips keepalive dispatch when workflow source context opts out of automation. |
| function extractExplicitIssueSyncNumbers(pr = {}) { | ||
| const text = `${pr.title || ''}\n${pr.body || ''}`; | ||
| const issueNumbers = new Set(); | ||
| const patterns = [ | ||
| /\b(?:close[sd]?|closing|fix(?:e[sd])?|fixing|resolve[sd]?|resolving|address(?:e[sd])?|addressing)\s*[:#-]?\s*#([0-9]+)\b/gi, | ||
| /\b(?:(?:relate[sd]?\s+to|refs?|references?)\s+(?:issue\s+)?|(?:source|github|linked)\s+issue\s*)[:#-]?\s*#([0-9]+)\b/gi, | ||
| ]; | ||
| for (const pattern of patterns) { | ||
| for (const match of text.matchAll(pattern)) { | ||
| issueNumbers.add(Number(match[1])); | ||
| } | ||
| } | ||
| return issueNumbers; | ||
| } | ||
|
|
||
| function hasExplicitIssueSyncReference(pr = {}) { | ||
| return extractExplicitIssueSyncNumbers(pr).size > 0; | ||
| } | ||
|
|
||
| function resolveNonIssueWorkflowSourceContextForBodySync(pr = {}, issueNumber = null) { | ||
| const explicitNonIssueSourceContext = resolveExplicitNonIssueWorkflowSourceContext(pr); | ||
| if (!explicitNonIssueSourceContext) { | ||
| return null; | ||
| } | ||
| const explicitIssueNumbers = extractExplicitIssueSyncNumbers(pr); | ||
| if (issueNumber && explicitIssueNumbers.has(Number(issueNumber))) { | ||
| return null; | ||
| } | ||
| return explicitNonIssueSourceContext; | ||
| } |
There was a problem hiding this comment.
resolveNonIssueWorkflowSourceContextForBodySync tries to stop treating a PR as “explicit non-issue” once it’s linked to an issue, but extractExplicitIssueSyncNumbers doesn’t detect the <!-- meta:issue:123 --> marker (which this repo’s own guidance recommends). As a result, a PR can still be skipped for issue-sourced body sync even when issueNumber came from the meta marker. Consider including the meta marker in extractExplicitIssueSyncNumbers (or directly short-circuiting when the extracted issueNumber is from the meta marker) so explicit issue linking via meta markers is honored.
| if (sourceContext.noAutomation) { | ||
| outputs.reason = 'no-automation-source-context'; | ||
| outputs.dispatch = 'false'; | ||
| core.info( | ||
| `Keepalive dispatch skipped: PR source context opts out of automation (${formatSourceContextForLog(sourceContext)}).`, | ||
| ); | ||
| return finalise(); | ||
| } |
There was a problem hiding this comment.
The noAutomation early-exit happens after the script has already fetched/created reactions and potentially added the lock reaction. If noAutomation is meant to opt out of all automation, consider checking sourceContext.noAutomation immediately after resolvePrSourceContext(pull) (before any reaction reads/writes) to avoid unnecessary API calls and leaving automation/lock reactions on PRs that opted out.
| const priorityArtifacts = await collectPriorityWorkflowArtifacts({ | ||
| github, | ||
| owner, | ||
| repo, | ||
| withRetry, | ||
| options, | ||
| }); | ||
| return dedupeArtifacts([...artifacts, ...priorityArtifacts]); |
There was a problem hiding this comment.
collectRepoArtifacts now always calls collectPriorityWorkflowArtifacts, which can add a significant number of extra Actions API calls (listWorkflowRuns + listWorkflowRunArtifacts per source) even when the initial repo-level artifact scan already contains all needed families. Consider (1) allowing priority_workflow_runs_per_source to be 0 to disable this pass, and/or (2) short-circuiting the priority scan when the repo artifacts already satisfy the families for a given source (or all priority families). This will reduce rate-limit pressure and speed up weekly metrics runs.
| } | ||
| return dedupeArtifacts(artifacts); | ||
| } | ||
|
|
There was a problem hiding this comment.
New selection logic is introduced here (dedupeArtifacts, familiesSatisfied, collectPriorityWorkflowArtifacts) and it affects which artifacts get downloaded (and how many API calls are made). Given the repo convention of unit-testing workflow helper scripts under .github/scripts/__tests__, please add minimal tests covering: deduping behavior, cutoff handling in familiesSatisfied, and that collectPriorityWorkflowArtifacts stops once required families are found / skips 404 workflows.
| module.exports.dedupeArtifacts = dedupeArtifacts; | |
| module.exports.familiesSatisfied = familiesSatisfied; | |
| module.exports.collectPriorityWorkflowArtifacts = collectPriorityWorkflowArtifacts; |
| function hasNoAutomationWorkflowContext(pull = {}) { | ||
| const body = String(pull?.body || ''); | ||
| const markerToken = normalizeToken(parseHtmlMarker(body, 'workflow-source')); | ||
| const block = parseWorkflowSourceBlock(body); | ||
| const blockTokens = [ | ||
| block.origin, | ||
| block.source, | ||
| block.type, | ||
| block.automation, | ||
| block.automation_intent, | ||
| ].map(normalizeToken); | ||
| const labels = labelNames(pull).map((label) => label.toLowerCase()); | ||
|
|
||
| return ( | ||
| markerToken === 'no_automation' | ||
| || blockTokens.includes('no_automation') | ||
| || labels.some((label) => NO_AUTOMATION_LABELS.has(label) || NO_AUTOMATION_LABELS.has(normalizeToken(label))) | ||
| || hasCheckedNoAutomationTemplate(body) | ||
| ); | ||
| } |
There was a problem hiding this comment.
hasNoAutomationWorkflowContext/hasCheckedNoAutomationTemplate introduce new parsing paths (labels, HTML markers, and checked-template detection) that now gate automation via hasValidNonIssueSourceContext and downstream workflows. Please add lightweight unit tests under .github/scripts/__tests__ to cover: label-based detection (workflow:no-automation), marker/block token detection, and the checked “Do not automate” template case (including the new Started from: sub-section parsing).
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Source SHA:
8a344d64eb1ca1e2318692e5f4c17c45415c7dccTemplate hash:
20d0e241bca2Sync branch:
sync/workflows-20d0e241bca2Consumer repo:
stranske/Trend_Model_ProjectManifest:
.github/sync-manifest.yml