From 6df19fa0b6c92b0564c2ef5bcaaf8a869ee9633c Mon Sep 17 00:00:00 2001 From: Tim Stranske Date: Mon, 27 Apr 2026 01:31:05 -0500 Subject: [PATCH] Fix issue metadata sync follow-ups --- .../agents-pr-meta-keepalive.test.js | 11 +- .../agents-pr-meta-update-body.test.js | 12 +- .../scripts/__tests__/source-context.test.js | 9 ++ .../weekly-metrics-artifacts.test.js | 117 ++++++++++++++++++ .github/scripts/agents_pr_meta_keepalive.js | 18 +-- .github/scripts/agents_pr_meta_update_body.js | 16 ++- .github/scripts/weekly_metrics_artifacts.js | 17 ++- .../scripts/agents_pr_meta_keepalive.js | 18 +-- .../scripts/agents_pr_meta_update_body.js | 16 ++- .../scripts/weekly_metrics_artifacts.js | 17 ++- 10 files changed, 216 insertions(+), 35 deletions(-) diff --git a/.github/scripts/__tests__/agents-pr-meta-keepalive.test.js b/.github/scripts/__tests__/agents-pr-meta-keepalive.test.js index 5b21586b54..aa0b7094ff 100644 --- a/.github/scripts/__tests__/agents-pr-meta-keepalive.test.js +++ b/.github/scripts/__tests__/agents-pr-meta-keepalive.test.js @@ -472,6 +472,7 @@ test('keepalive detection accepts valid non-issue source context', async () => { test('keepalive detection blocks no-automation source context', async () => { const outputs = {}; + const reactionCalls = []; const scopeBlock = [ '', '', @@ -508,10 +509,12 @@ test('keepalive detection blocks no-automation source context', async () => { }, reactions: { async listForIssueComment() { - return { data: [] }; + reactionCalls.push('list'); + throw new Error('no-automation PRs should not read reactions'); }, async createForIssueComment() { - return { status: 201, data: { content: 'hooray' } }; + reactionCalls.push('create'); + throw new Error('no-automation PRs should not write reactions'); }, }, }, @@ -520,7 +523,8 @@ test('keepalive detection blocks no-automation source context', async () => { return []; } if (method === this.rest.reactions.listForIssueComment) { - return []; + reactionCalls.push('paginate-reactions'); + throw new Error('no-automation PRs should not paginate reactions'); } return []; }, @@ -551,6 +555,7 @@ test('keepalive detection blocks no-automation source context', async () => { assert.equal(outputs.dispatch, 'false'); assert.equal(outputs.reason, 'no-automation-source-context'); assert.equal(outputs.source_type, 'manual_remote'); + assert.deepEqual(reactionCalls, []); }); test('keepalive detection accepts sync campaign source context without linked issue', async () => { diff --git a/.github/scripts/__tests__/agents-pr-meta-update-body.test.js b/.github/scripts/__tests__/agents-pr-meta-update-body.test.js index 6013c1e025..8500fca264 100644 --- a/.github/scripts/__tests__/agents-pr-meta-update-body.test.js +++ b/.github/scripts/__tests__/agents-pr-meta-update-body.test.js @@ -518,7 +518,7 @@ test('resolveNonIssueWorkflowSourceContextForBodySync preserves explicit issue-s assert.equal(context, null); }); -test('resolveNonIssueWorkflowSourceContextForBodySync lets explicit non-issue markers replace generated issue metadata', () => { +test('resolveNonIssueWorkflowSourceContextForBodySync yields to meta issue markers', () => { const context = resolveNonIssueWorkflowSourceContextForBodySync( { body: [ @@ -530,8 +530,7 @@ test('resolveNonIssueWorkflowSourceContextForBodySync lets explicit non-issue ma 123, ); - assert.equal(context.sourceType, 'local_request'); - assert.equal(context.sourceRef, 'codex-thread-2026-04-26'); + assert.equal(context, null); }); test('hasExplicitIssueSyncReference ignores heuristic issue-number sources', () => { @@ -546,21 +545,22 @@ test('hasExplicitIssueSyncReference ignores heuristic issue-number sources', () assert.equal(hasExplicitIssueSyncReference({ body: 'Closes #123' }), true); assert.equal(hasExplicitIssueSyncReference({ body: 'Related to #123' }), true); assert.equal(hasExplicitIssueSyncReference({ body: 'Related to issue #123' }), true); + assert.equal(hasExplicitIssueSyncReference({ body: 'Related to campaign issue #123' }), true); assert.equal(hasExplicitIssueSyncReference({ body: 'Refs #123' }), true); assert.equal(hasExplicitIssueSyncReference({ body: 'References issue #123' }), true); assert.equal(hasExplicitIssueSyncReference({ body: 'source issue #123' }), true); assert.equal(hasExplicitIssueSyncReference({ body: 'This issue only mentions review follow-up PR #123' }), false); assert.equal(hasExplicitIssueSyncReference({ body: 'refs in this paragraph mention PR #123' }), false); - assert.equal(hasExplicitIssueSyncReference({ body: '' }), false); + assert.equal(hasExplicitIssueSyncReference({ body: '' }), true); }); test('extractExplicitIssueSyncNumbers returns only explicit issue references', () => { assert.deepEqual( Array.from(extractExplicitIssueSyncNumbers({ title: 'Review follow-up', - body: 'Closes #123\nReferences issue #456\nRefs #234\nReview follow-up from PR #789', + body: 'Closes #123\nReferences issue #456\nRefs #234\nRelated to campaign issue #345\n\nReview follow-up from PR #789', })).sort((a, b) => a - b), - [123, 234, 456], + [123, 234, 345, 456, 567], ); }); diff --git a/.github/scripts/__tests__/source-context.test.js b/.github/scripts/__tests__/source-context.test.js index 9b50cf3e1f..c0a398056e 100644 --- a/.github/scripts/__tests__/source-context.test.js +++ b/.github/scripts/__tests__/source-context.test.js @@ -211,6 +211,15 @@ Automation intent: assert.equal(context.sourceType, SOURCE_TYPES.MANUAL_REMOTE); assert.equal(context.noAutomation, true); assert.equal(hasNoAutomationWorkflowContext({ labels: [{ name: 'workflow:no-automation' }] }), true); + assert.equal(hasNoAutomationWorkflowContext({ body: '' }), true); + assert.equal(hasNoAutomationWorkflowContext({ + body: ` + +origin: local_request +automation: no_automation + +`, + }), true); }); test('sourceTypeFromCheckedTemplate rejects ambiguous checked source choices', () => { diff --git a/.github/scripts/__tests__/weekly-metrics-artifacts.test.js b/.github/scripts/__tests__/weekly-metrics-artifacts.test.js index d624fdada8..e00ee89bd8 100644 --- a/.github/scripts/__tests__/weekly-metrics-artifacts.test.js +++ b/.github/scripts/__tests__/weekly-metrics-artifacts.test.js @@ -14,6 +14,7 @@ const { latestCandidateByFamily, missingPriorityFamilies, normalizeSelectionOptions, + PRIORITY_METRICS_FAMILIES, priorityFamilyStatuses, selectMetricsArtifacts, } = require('../weekly_metrics_artifacts.js'); @@ -418,6 +419,12 @@ test('normalizes invalid environment-like limits to defaults', () => { assert.equal(options.max_per_family, 20); assert.equal(options.max_scan_pages, 5); assert.equal(options.priority_workflow_runs_per_source, 10); + + const disabledPriorityOptions = normalizeSelectionOptions({ + now_ms: NOW, + priority_workflow_runs_per_source: '0', + }); + assert.equal(disabledPriorityOptions.priority_workflow_runs_per_source, 0); }); test('deduplicates artifacts by stable id before selection', () => { @@ -519,6 +526,73 @@ test('collects priority artifacts from their producer workflows', async () => { ]); }); +test('collects priority artifacts until each source satisfies its own families', async () => { + const calls = []; + const client = { + rest: { + actions: { + listWorkflowRuns: async (params) => { + calls.push(['runs', params.workflow_id]); + return { + data: { + workflow_runs: params.workflow_id === 'source-a.yml' + ? [ + { id: 101, created_at: '2026-04-25T11:00:00Z', updated_at: '2026-04-25T11:00:00Z' }, + ] + : [ + { id: 201, created_at: '2026-04-25T11:00:00Z', updated_at: '2026-04-25T11:00:00Z' }, + { id: 202, created_at: '2026-04-25T10:00:00Z', updated_at: '2026-04-25T10:00:00Z' }, + ], + }, + }; + }, + listWorkflowRunArtifacts: async (params) => { + calls.push(['artifacts', params.run_id]); + return { + data: { + artifacts: [ + artifact( + params.run_id, + params.run_id === 201 ? 'keepalive-metrics' : `codex-cli-freshness-${params.run_id}`, + '2026-04-25T11:00:00Z' + ), + ], + }, + }; + }, + }, + }, + }; + + const artifacts = await collectPriorityWorkflowArtifacts({ + github: client, + owner: 'owner', + repo: 'repo', + options: { + now_ms: NOW, + priority_workflow_runs_per_source: 2, + per_page: 50, + }, + sources: [ + { workflow_id: 'source-a.yml', families: ['codex-cli-freshness'] }, + { workflow_id: 'source-b.yml', families: ['codex-cli-freshness'] }, + ], + withRetry: (fn) => fn(client), + }); + + assert.deepEqual( + artifacts.map((selected) => selected.name), + ['codex-cli-freshness-101', 'codex-cli-freshness-202'] + ); + assert.deepEqual(calls, [ + ['runs', 'source-a.yml'], + ['artifacts', 101], + ['runs', 'source-b.yml'], + ['artifacts', 201], + ['artifacts', 202], + ]); +}); + test('skips missing priority producer workflows without failing selection', async () => { const client = { rest: { @@ -607,3 +681,46 @@ test('collects repo artifacts within the configured scan page cap', async () => ); assert.ok(calls.every((call) => call.per_page === 2)); }); + +test('skips priority workflow scan when repo artifacts already satisfy priority families', async () => { + const calls = []; + const client = { + rest: { + actions: { + listArtifactsForRepo: async (params) => { + calls.push(['repo-artifacts', params.page]); + return { + data: { + artifacts: PRIORITY_METRICS_FAMILIES.map((family, index) => + artifact( + index + 1, + family === 'pr-source-context' ? family : `${family}-current`, + '2026-04-25T11:00:00Z' + ) + ), + }, + }; + }, + listWorkflowRuns: async () => { + calls.push(['runs']); + throw new Error('priority scan should be skipped'); + }, + }, + }, + }; + + const artifacts = await collectRepoArtifacts({ + github: client, + owner: 'owner', + repo: 'repo', + options: { + now_ms: NOW, + max_scan_pages: 1, + per_page: 100, + }, + withRetry: (fn) => fn(client), + }); + + assert.deepEqual(calls, [['repo-artifacts', 1]]); + assert.equal(artifacts.length, PRIORITY_METRICS_FAMILIES.length); +}); diff --git a/.github/scripts/agents_pr_meta_keepalive.js b/.github/scripts/agents_pr_meta_keepalive.js index 2e241bf4f2..7b286af1ec 100644 --- a/.github/scripts/agents_pr_meta_keepalive.js +++ b/.github/scripts/agents_pr_meta_keepalive.js @@ -661,6 +661,15 @@ async function detectKeepalive({ core, github, context, env = process.env }) { outputs.source_ref = sourceContext.sourceRef; } + 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(); + } + let reactions = []; try { reactions = await github.paginate(github.rest.reactions.listForIssueComment, { @@ -755,15 +764,6 @@ async function detectKeepalive({ core, github, context, env = process.env }) { return finalise(); } - 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(); - } - if (!issueNumber) { if (sourceContext.isValid && !sourceContext.requiresIssue) { core.info( diff --git a/.github/scripts/agents_pr_meta_update_body.js b/.github/scripts/agents_pr_meta_update_body.js index 47e6c4df45..d84f89d474 100644 --- a/.github/scripts/agents_pr_meta_update_body.js +++ b/.github/scripts/agents_pr_meta_update_body.js @@ -980,7 +980,11 @@ async function createIssueCommentWithRetry({ github, owner, repo, issueNumber, b } function buildSourceContextResolvedCommentBody(prNumber, sourceContext) { - const isIssueBacked = sourceContext?.requiresIssue || sourceContext?.issueNumber || sourceContext?.sourceType === SOURCE_TYPES.GITHUB_ISSUE; + const isIssueBacked = Boolean( + sourceContext?.requiresIssue + || sourceContext?.issueNumber + || sourceContext?.sourceType === SOURCE_TYPES.GITHUB_ISSUE + ); return [ '', '### Workflow source detected', @@ -1030,8 +1034,9 @@ function extractExplicitIssueSyncNumbers(pr = {}) { const text = `${pr.title || ''}\n${pr.body || ''}`; const issueNumbers = new Set(); const patterns = [ + //gi, /\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, + /\b(?:(?:relate[sd]?\s+to|refs?|references?)\s+(?:(?:[a-z-]+\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)) { @@ -1392,6 +1397,13 @@ async function run({github: rawGithub, context, core, inputs}) { const issueNumber = extractIssueNumberFromPull(pr); const sourceContext = resolvePrSourceContext(pr); + if (sourceContext.noAutomation) { + core.info( + `PR #${pr.number} has automation disabled (${formatSourceContextForLog(sourceContext)}); skipping PR body update and automation comment management.`, + ); + return; + } + const explicitNonIssueSourceContext = resolveNonIssueWorkflowSourceContextForBodySync(pr, issueNumber); if (explicitNonIssueSourceContext) { core.info( diff --git a/.github/scripts/weekly_metrics_artifacts.js b/.github/scripts/weekly_metrics_artifacts.js index adc46b29bf..8cc1177900 100644 --- a/.github/scripts/weekly_metrics_artifacts.js +++ b/.github/scripts/weekly_metrics_artifacts.js @@ -87,6 +87,11 @@ function parsePositiveInt(value, fallback) { return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; } +function parseNonNegativeInt(value, fallback) { + const parsed = Number.parseInt(cleanString(value), 10); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : fallback; +} + function parseDateMs(value) { const text = cleanString(value); if (!text) return 0; @@ -134,7 +139,7 @@ function normalizeSelectionOptions(options = {}) { options.per_page ?? options.perPage ?? process.env.METRICS_ARTIFACTS_PER_PAGE, DEFAULT_PER_PAGE ); - const priorityWorkflowRunsPerSource = parsePositiveInt( + const priorityWorkflowRunsPerSource = parseNonNegativeInt( options.priority_workflow_runs_per_source ?? options.priorityWorkflowRunsPerSource ?? process.env.METRICS_PRIORITY_WORKFLOW_RUNS_PER_SOURCE, @@ -491,10 +496,14 @@ async function collectPriorityWorkflowArtifacts({ }) { const config = normalizeSelectionOptions(options); const artifacts = []; + if (config.priority_workflow_runs_per_source <= 0) { + return artifacts; + } for (const source of sources) { const workflowId = cleanString(source.workflow_id ?? source.workflowId); const families = (source.families || []).map(cleanString).filter(Boolean); if (!workflowId || families.length === 0) continue; + const sourceArtifacts = []; let runsResponse; try { runsResponse = await withRetry((client) => client.rest.actions.listWorkflowRuns({ @@ -533,8 +542,9 @@ async function collectPriorityWorkflowArtifacts({ const matchingArtifacts = (artifactResponse?.data?.artifacts || []).filter((artifact) => families.includes(artifactFamily(artifact.name)) ); + sourceArtifacts.push(...matchingArtifacts); artifacts.push(...matchingArtifacts); - if (familiesSatisfied(artifacts, families, config)) { + if (familiesSatisfied(sourceArtifacts, families, config)) { break; } } @@ -565,6 +575,9 @@ async function collectRepoArtifacts({ github, owner, repo, withRetry, options }) break; } } + if (familiesSatisfied(artifacts, PRIORITY_METRICS_FAMILIES, config)) { + return dedupeArtifacts(artifacts); + } const priorityArtifacts = await collectPriorityWorkflowArtifacts({ github, owner, diff --git a/templates/consumer-repo/.github/scripts/agents_pr_meta_keepalive.js b/templates/consumer-repo/.github/scripts/agents_pr_meta_keepalive.js index 2e241bf4f2..7b286af1ec 100644 --- a/templates/consumer-repo/.github/scripts/agents_pr_meta_keepalive.js +++ b/templates/consumer-repo/.github/scripts/agents_pr_meta_keepalive.js @@ -661,6 +661,15 @@ async function detectKeepalive({ core, github, context, env = process.env }) { outputs.source_ref = sourceContext.sourceRef; } + 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(); + } + let reactions = []; try { reactions = await github.paginate(github.rest.reactions.listForIssueComment, { @@ -755,15 +764,6 @@ async function detectKeepalive({ core, github, context, env = process.env }) { return finalise(); } - 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(); - } - if (!issueNumber) { if (sourceContext.isValid && !sourceContext.requiresIssue) { core.info( diff --git a/templates/consumer-repo/.github/scripts/agents_pr_meta_update_body.js b/templates/consumer-repo/.github/scripts/agents_pr_meta_update_body.js index 47e6c4df45..d84f89d474 100644 --- a/templates/consumer-repo/.github/scripts/agents_pr_meta_update_body.js +++ b/templates/consumer-repo/.github/scripts/agents_pr_meta_update_body.js @@ -980,7 +980,11 @@ async function createIssueCommentWithRetry({ github, owner, repo, issueNumber, b } function buildSourceContextResolvedCommentBody(prNumber, sourceContext) { - const isIssueBacked = sourceContext?.requiresIssue || sourceContext?.issueNumber || sourceContext?.sourceType === SOURCE_TYPES.GITHUB_ISSUE; + const isIssueBacked = Boolean( + sourceContext?.requiresIssue + || sourceContext?.issueNumber + || sourceContext?.sourceType === SOURCE_TYPES.GITHUB_ISSUE + ); return [ '', '### Workflow source detected', @@ -1030,8 +1034,9 @@ function extractExplicitIssueSyncNumbers(pr = {}) { const text = `${pr.title || ''}\n${pr.body || ''}`; const issueNumbers = new Set(); const patterns = [ + //gi, /\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, + /\b(?:(?:relate[sd]?\s+to|refs?|references?)\s+(?:(?:[a-z-]+\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)) { @@ -1392,6 +1397,13 @@ async function run({github: rawGithub, context, core, inputs}) { const issueNumber = extractIssueNumberFromPull(pr); const sourceContext = resolvePrSourceContext(pr); + if (sourceContext.noAutomation) { + core.info( + `PR #${pr.number} has automation disabled (${formatSourceContextForLog(sourceContext)}); skipping PR body update and automation comment management.`, + ); + return; + } + const explicitNonIssueSourceContext = resolveNonIssueWorkflowSourceContextForBodySync(pr, issueNumber); if (explicitNonIssueSourceContext) { core.info( diff --git a/templates/consumer-repo/.github/scripts/weekly_metrics_artifacts.js b/templates/consumer-repo/.github/scripts/weekly_metrics_artifacts.js index adc46b29bf..8cc1177900 100644 --- a/templates/consumer-repo/.github/scripts/weekly_metrics_artifacts.js +++ b/templates/consumer-repo/.github/scripts/weekly_metrics_artifacts.js @@ -87,6 +87,11 @@ function parsePositiveInt(value, fallback) { return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; } +function parseNonNegativeInt(value, fallback) { + const parsed = Number.parseInt(cleanString(value), 10); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : fallback; +} + function parseDateMs(value) { const text = cleanString(value); if (!text) return 0; @@ -134,7 +139,7 @@ function normalizeSelectionOptions(options = {}) { options.per_page ?? options.perPage ?? process.env.METRICS_ARTIFACTS_PER_PAGE, DEFAULT_PER_PAGE ); - const priorityWorkflowRunsPerSource = parsePositiveInt( + const priorityWorkflowRunsPerSource = parseNonNegativeInt( options.priority_workflow_runs_per_source ?? options.priorityWorkflowRunsPerSource ?? process.env.METRICS_PRIORITY_WORKFLOW_RUNS_PER_SOURCE, @@ -491,10 +496,14 @@ async function collectPriorityWorkflowArtifacts({ }) { const config = normalizeSelectionOptions(options); const artifacts = []; + if (config.priority_workflow_runs_per_source <= 0) { + return artifacts; + } for (const source of sources) { const workflowId = cleanString(source.workflow_id ?? source.workflowId); const families = (source.families || []).map(cleanString).filter(Boolean); if (!workflowId || families.length === 0) continue; + const sourceArtifacts = []; let runsResponse; try { runsResponse = await withRetry((client) => client.rest.actions.listWorkflowRuns({ @@ -533,8 +542,9 @@ async function collectPriorityWorkflowArtifacts({ const matchingArtifacts = (artifactResponse?.data?.artifacts || []).filter((artifact) => families.includes(artifactFamily(artifact.name)) ); + sourceArtifacts.push(...matchingArtifacts); artifacts.push(...matchingArtifacts); - if (familiesSatisfied(artifacts, families, config)) { + if (familiesSatisfied(sourceArtifacts, families, config)) { break; } } @@ -565,6 +575,9 @@ async function collectRepoArtifacts({ github, owner, repo, withRetry, options }) break; } } + if (familiesSatisfied(artifacts, PRIORITY_METRICS_FAMILIES, config)) { + return dedupeArtifacts(artifacts); + } const priorityArtifacts = await collectPriorityWorkflowArtifacts({ github, owner,