chore: sync workflow templates - #132
Conversation
Automated sync from stranske/Workflows Template hash: 93ee083443e7 Changes synced from sync-manifest.yml
stranske
added
sync
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Status | ✅ no new diagnostics |
|
Autofix updated these files:
|
🤖 Keepalive Loop StatusPR #132 | Agent: Codex | Iteration 0/5 Current State
🔍 Failure Classification| Error type | infrastructure | |
There was a problem hiding this comment.
Pull request overview
This PR syncs workflow templates from the central Workflows repository, significantly expanding the agents-verify-to-issue.yml workflow to extract and analyze multiple data sources when creating follow-up issues from verification feedback.
Key changes:
- Enhanced data extraction to parse provider comparison reports, keepalive state, and PR body content
- Added structural problem detection to identify issues with task formatting and agent productivity
- Implemented concern deduplication and prioritization across multiple providers
- Restructured issue body format to present verification analysis, agent history, and actionable tasks
| const isNonActionable = (task) => { | ||
| return nonActionablePatterns.some(p => p.test(task.trim())) || | ||
| task.length < 10 || | ||
| task.length > 150; |
There was a problem hiding this comment.
The task length validation uses arbitrary thresholds (< 10 or > 150 characters) without clear justification. Tasks between 10-150 characters could still be code snippets or non-actionable items (e.g., "def function_name(param1, param2, param3):"). Consider adding more semantic checks rather than relying solely on length, or document why these specific thresholds were chosen.
| const isNonActionable = (task) => { | |
| return nonActionablePatterns.some(p => p.test(task.trim())) || | |
| task.length < 10 || | |
| task.length > 150; | |
| // Heuristic to detect code-like content that may not be covered by the regexes above. | |
| // Looks for common code symbols/operators that are unlikely to appear in natural-language tasks. | |
| const isProbablyCodeLike = (task) => { | |
| const codeLikePatterns = [ | |
| /[{}();]/, // Braces, parens, semicolons | |
| /::|=>/, // Common in many languages | |
| /<\/\w+>/, // HTML / XML closing tags | |
| ]; | |
| const trimmed = task.trim(); | |
| return codeLikePatterns.some(p => p.test(trimmed)); | |
| }; | |
| const isNonActionable = (task) => { | |
| const trimmed = task.trim(); | |
| const wordCount = trimmed === '' ? 0 : trimmed.split(/\s+/).length; | |
| return ( | |
| // Known non-actionable patterns (code, types, comments, etc.) | |
| nonActionablePatterns.some(p => p.test(trimmed)) || | |
| // Very short items (fewer than 3 words) are rarely actionable tasks. | |
| wordCount > 0 && wordCount < 3 || | |
| // Very long items that are also code-like are likely snippets, not tasks. | |
| (trimmed.length > 150 && isProbablyCodeLike(trimmed)) | |
| ); |
| .sort((a, b) => (b.sources.length - a.sources.length)) | ||
| .map(c => { | ||
| const sourceTag = c.sources.length > 1 | ||
| ? ' *(agreed by both providers)*' : ''; |
There was a problem hiding this comment.
The sourceTag generation on lines 314-315 uses a binary check (sources.length > 1) and displays "agreed by both providers". This assumes exactly 2 providers. If there are 3+ providers that agree on a concern, the message would be misleading. Consider using "agreed by X providers" or "agreed by multiple providers" for accuracy.
| ? ' *(agreed by both providers)*' : ''; | |
| ? ' *(agreed by ' + c.sources.length + ' providers)*' : ''; |
|
|
||
| // Extract individual provider details from expandable section | ||
| const providerMatches = [...body.matchAll( | ||
| /#### (\w+(?:-\w+)?)\n/g |
There was a problem hiding this comment.
The regex pattern /#### (\w+(?:-\w+)?)\n/g may not match provider names that contain multiple hyphens or other special characters. For example, if a provider name is "provider-name-v2" this would only match "provider-name". Consider using a more flexible pattern like /#### ([\w-]+)\n/g to match one or more word characters or hyphens.
| /#### (\w+(?:-\w+)?)\n/g | |
| /#### ([\w-]+)\n/g |
| /^```\w*$/, // Code fence markers | ||
| /^class\s+\w+/, // Class definitions | ||
| /^def\s+\w+/, // Function definitions | ||
| /^\w+:\s*(str|int|float|bool|Any|list|dict|None)/i, // Type annotations |
There was a problem hiding this comment.
The pattern /^\w+:\s*(str|int|float|bool|Any|list|dict|None)/i on line 156 is checking for Python type annotations, but the case-insensitive flag i could cause false positives. For example, "STRING: value" would match when it shouldn't. Python type annotations are case-sensitive, so the i flag should be removed.
| /^\w+:\s*(str|int|float|bool|Any|list|dict|None)/i, // Type annotations | |
| /^\w+:\s*(str|int|float|bool|Any|list|dict|None)/, // Type annotations |
| priority: provider.concerns.indexOf(concern) === 0 ? 'high' : 'normal' | ||
| }); | ||
| } else { | ||
| allConcerns.get(key).sources.push(provider.name); | ||
| allConcerns.get(key).priority = 'high'; // Multiple providers = high priority |
There was a problem hiding this comment.
The priority assignment logic is inconsistent. On line 285, the priority is set to 'high' for the first concern, otherwise 'normal'. But on line 289, when a concern is found in multiple providers, it's always set to 'high'. This means a concern that wasn't first for any provider but appears in multiple providers would override the priority to 'high', but a first concern from a single provider would remain 'high'. The comment says "Multiple providers = high priority" but the logic doesn't match - it should check if sources.length > 1, not just set it unconditionally.
| // Filter out code snippets that aren't real tasks | ||
| if (!task.match(/^```|^class |^def |^\w+:\s*\w+\s*$|^#|^\s*$/) && |
There was a problem hiding this comment.
The duplicate filtering logic on line 256 has a slightly different regex pattern than the one used in the isNonActionable function. Line 256 uses /^```|^class |^def |^\w+:\s*\w+\s*$|^#|^\s*$/ while the isNonActionable function uses more comprehensive patterns. This inconsistency could lead to different items being filtered at different stages. Consider using the same isNonActionable function here for consistency.
| // Filter out code snippets that aren't real tasks | |
| if (!task.match(/^```|^class |^def |^\w+:\s*\w+\s*$|^#|^\s*$/) && | |
| // Filter out code snippets and other non-actionable lines | |
| if (!isNonActionable(task) && |
| for (const provider of providerData.providers) { | ||
| for (const concern of provider.concerns) { | ||
| // Normalize for deduplication | ||
| const key = concern.toLowerCase().replace(/[^a-z0-9]/g, '').substring(0, 50); |
There was a problem hiding this comment.
The concern deduplication logic uses only the first 50 characters of a normalized concern for the key (line 280). This could cause different concerns to be incorrectly merged if they share the same first 50 characters after normalization. For example, "Missing test coverage for authentication module" and "Missing test coverage for authorization handlers" would have very similar prefixes. Consider using the full normalized string or a hash function for more accurate deduplication.
| const key = concern.toLowerCase().replace(/[^a-z0-9]/g, '').substring(0, 50); | |
| const key = concern.toLowerCase().replace(/[^a-z0-9]/g, ''); |
| const concernTasks = [...allConcerns.values()] | ||
| .sort((a, b) => (b.sources.length - a.sources.length)) | ||
| .map(c => { | ||
| const sourceTag = c.sources.length > 1 | ||
| ? ' *(agreed by both providers)*' : ''; | ||
| return '- [ ] ' + c.text + sourceTag; |
There was a problem hiding this comment.
The 'priority' field is computed and stored in the concerns map but is never used in the output. The concerns are sorted by sources.length (line 312) but not by priority. Either use the priority field in sorting/output, or remove it to avoid confusion.
| const concernsPattern = '#### ' + providerName + | ||
| '[\\s\\S]*?- \\*\\*Concerns:\\*\\*' + | ||
| '([\\s\\S]*?)(?=####|### |$)'; | ||
| const providerSection = body.match(new RegExp(concernsPattern)); | ||
| if (providerSection) { | ||
| const concernRegex = /^\s*-\s+(.+)$/gm; | ||
| const concernLines = providerSection[1].match(concernRegex) || []; | ||
| provider.concerns = concernLines.map( | ||
| l => l.replace(/^\s*-\s+/, '').trim() | ||
| ); | ||
| } | ||
|
|
||
| // Extract scores | ||
| const scoresPattern = '#### ' + providerName + | ||
| '[\\s\\S]*?- \\*\\*Scores:\\*\\*' + | ||
| '([\\s\\S]*?)(?=- \\*\\*Summary|####|$)'; | ||
| const scoresMatch = body.match(new RegExp(scoresPattern)); |
There was a problem hiding this comment.
Similar issue as the previous comment - the provider name is used in a regex pattern without escaping. This appears in three places (lines 97-100, 110-113). If provider names contain regex special characters, the RegExp constructor will throw an error or produce incorrect matches.
| providerData.uniqueInsights = insightsMatch[1].trim(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The old error handling logic that called core.setFailed() when no verification comment was found has been removed. The new code silently continues with empty data if no provider comparison is found (line 69 onwards). This means the workflow will create an issue with "No provider comparison found" text even when there's no verification data at all. Consider adding a check to fail early if no verification data is available, similar to the old behavior.
| // If we couldn't find any verification comparison data, fail early | |
| if ( | |
| !comparisonComment || | |
| ( | |
| (!providerData.providers || providerData.providers.length === 0) && | |
| !providerData.agreement && | |
| !providerData.uniqueInsights | |
| ) | |
| ) { | |
| core.setFailed('No verification comparison found in verification comments; aborting issue creation.'); | |
| return; | |
| } |
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Manifest:
.github/sync-manifest.yml