Conversation
Automated sync from stranske/Workflows Template hash: abfacabe3e5b Changes synced from sync-manifest.yml
stranske
added
sync
stranske
temporarily deployed
to
agent-standard
GitHub Actions
Inactive
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
stranske
temporarily deployed
to
agent-standard
GitHub Actions
Inactive
🤖 Keepalive Loop StatusPR #137 | Agent: Codex | Iteration 0/5 Current State
🔍 Failure Classification| Error type | infrastructure | |
|
Status | ✅ no new diagnostics |
There was a problem hiding this comment.
Pull request overview
This PR syncs workflow templates from the stranske/Workflows repository, adding LLM-based task completion analysis and sophisticated rate limit detection to the keepalive loop system.
Key Changes
- LLM Task Analysis Integration: Adds support for LLM-based task completion detection with provider tracking (GitHub Models, OpenAI, regex fallback) and confidence scoring, integrated into the auto-reconciliation workflow
- Rate Limit Detection: Implements intelligent detection of gate cancellations due to rate limits by inspecting job annotations and logs, enabling automatic deferral with retry logic
- Enhanced Status Display: Introduces disposition labels and improved summary formatting to clearly distinguish between transient failures, deferrals, and actual errors
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
.github/workflows/agents-keepalive-loop.yml |
Adds LLM completed tasks parsing and passes LLM provider metadata to summary update step |
.github/scripts/keepalive_loop.js |
Implements rate limit detection, LLM task integration in reconciliation, force retry mechanism, and enhanced summary display with disposition labels |
| const llmTasksLower = new Set((llmCompletedTasks || []).map(t => t.toLowerCase())); | ||
| const commitMatches = analysis.matches | ||
| .filter(m => m.confidence === 'high') | ||
| .filter(m => !llmTasksLower.has(m.task.toLowerCase())); |
There was a problem hiding this comment.
The code assumes all elements in llmCompletedTasks are strings when calling .toLowerCase() on line 1764. If the LLM analysis returns non-string values (e.g., objects, numbers, null), this will throw a TypeError. Consider adding validation or type coercion to ensure all task values are strings before processing.
| const llmTasksLower = new Set((llmCompletedTasks || []).map(t => t.toLowerCase())); | |
| const commitMatches = analysis.matches | |
| .filter(m => m.confidence === 'high') | |
| .filter(m => !llmTasksLower.has(m.task.toLowerCase())); | |
| const llmTasksLower = new Set( | |
| (llmCompletedTasks || []).map(t => normalise(t).toLowerCase()) | |
| ); | |
| const commitMatches = analysis.matches | |
| .filter(m => m.confidence === 'high') | |
| .filter(m => !llmTasksLower.has(normalise(m.task).toLowerCase())); |
| for (const task of llmCompletedTasks) { | ||
| highConfidence.push({ | ||
| task, | ||
| reason: 'LLM session analysis', | ||
| confidence: 'high', | ||
| source: 'llm', | ||
| }); | ||
| } |
There was a problem hiding this comment.
The code doesn't validate that tasks from llmCompletedTasks are strings before using them. If the array contains non-string values, operations like .slice() on line 1797 and regex matching on line 1792 will fail. Consider filtering or normalizing the tasks to strings using a helper function like String(task) or filtering out non-string values.
| for (const job of jobs) { | ||
| if (canCheckAnnotations) { | ||
| const checkRunId = extractCheckRunId(job); | ||
| if (checkRunId) { | ||
| const params = { | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| check_run_id: checkRunId, | ||
| per_page: 100, | ||
| }; | ||
| const annotations = github.paginate | ||
| ? await github.paginate(github.rest.checks.listAnnotations, params) | ||
| : (await github.rest.checks.listAnnotations(params))?.data; | ||
| if (annotationsContainRateLimit(annotations)) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (canCheckLogs) { | ||
| const jobId = Number(job?.id) || 0; | ||
| if (jobId) { | ||
| try { | ||
| const logs = await github.rest.actions.downloadJobLogsForWorkflowRun({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| job_id: jobId, | ||
| }); | ||
| if (logContainsRateLimit(logs?.data)) { | ||
| return true; | ||
| } | ||
| } catch (error) { | ||
| if (core) core.info(`Failed to inspect Gate job logs for rate limits: ${error.message}`); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The rate limit detection could make many API calls: for each job in the workflow run, it may call both listAnnotations (with pagination) and downloadJobLogsForWorkflowRun. For a gate workflow with many jobs, this could result in dozens of API calls and potentially hit rate limits itself. Consider:
- Checking annotations first for all jobs before falling back to logs (since logs are more expensive)
- Adding early termination after checking a few jobs if no rate limit signals are found
- Adding a comment explaining the performance tradeoff
| '### 🧠 Task Analysis', | ||
| `| Provider | ${providerIcon} ${providerLabel} |`, | ||
| `| Confidence | ${confidencePercent}% |`, | ||
| ); |
There was a problem hiding this comment.
The Task Analysis table is missing a header row and separator line, making it inconsistent with other tables in the summary (e.g., lines 1138-1139, 1164-1165). While this follows the same pattern as the Failure Classification section (line 1211), both should either include proper table headers and separators for consistency, or use a different format like a definition list or paragraph-style display.
|
|
||
| const RATE_LIMIT_PATTERNS = [ | ||
| /rate limit/i, | ||
| /rate[-\s]limit/i, |
There was a problem hiding this comment.
The pattern /rate[-\s]limit/i on line 642 is redundant as it will match the same strings as both /rate limit/i (line 641) and /rate[-\s]limited/i (line 643). Consider removing this redundant pattern to simplify the array.
| /rate[-\s]limit/i, |
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Manifest:
.github/sync-manifest.yml