fix: harden auto-pilot stall handoffs - #2826
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughThe auto-pilot workflows now preserve stall-rotation state, assess keepalive agents against available credentials and current assignment, and explicitly dispatch ordered keepalive handoff workflows when monitor stalls occur. ChangesStall rotation and keepalive handoff
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant monitor-pr
participant agent_stall_rotation.js
participant GitHubSecrets
participant KeepaliveWorkflows
monitor-pr->>agent_stall_rotation.js: load registry and current agent
monitor-pr->>GitHubSecrets: verify available repository secrets
GitHubSecrets-->>monitor-pr: secret names
monitor-pr->>agent_stall_rotation.js: compute eligible keepalive agents
agent_stall_rotation.js-->>monitor-pr: eligible alternatives
monitor-pr->>KeepaliveWorkflows: dispatch ordered handoff workflow
KeepaliveWorkflows-->>monitor-pr: dispatch result
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Workflow source neededPR #2826 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
Automated Status SummaryHead SHA: 1e671dc
Coverage Overview
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdeee7d91e
ℹ️ 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".
| await withRetry((client) => client.rest.actions.createWorkflowDispatch({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| workflow_id: 'agents-81-gate-followups.yml', | ||
| ref: repoInfo.default_branch || 'main', |
There was a problem hiding this comment.
Fall back to the follow-up workflow present in this repository
When this stall path runs in the Workflows repository itself, the dispatch targets a workflow that is absent: a repo-wide workflow search finds agents-81-gate-followups.yml only under templates/consumer-repo/, while the root still has .github/workflows/agents-keepalive-loop.yml. The API call therefore fails and the surrounding catch immediately applies needs-human, so monitor-PR handoff never works for this first-party repository; use the same first-available fallback already used by the create-PR path.
Useful? React with 👍 / 👎.
| HAS_CODEX_AUTH_JSON: ${{ secrets.CODEX_AUTH_JSON != '' }} | ||
| HAS_CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN != '' }} | ||
| HAS_CLAUDE_AUTH_JSON: ${{ secrets.CLAUDE_AUTH_JSON != '' }} | ||
| HAS_CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY != '' }} | ||
| HAS_GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY != '' }} |
There was a problem hiding this comment.
Derive credential checks from the agent registry
Replace this fixed provider-secret list with registry-derived availability: whenever a new pr_keepalive agent is enabled with a different required_secrets entry, eligibleAgents receives no value for that secret and silently excludes the otherwise configured agent from stall handoffs. This makes auto-pilot routing require another provider-specific workflow edit instead of growing with the registry as required.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
| secrets: availableSecrets, | ||
| }); | ||
| if (!hasAuto && keepaliveAgents.length > 1) { |
There was a problem hiding this comment.
Allow handoff when one viable alternative remains
When the stalled current agent is unavailable because its credential was removed while exactly one different agent remains credentialed, the new secret filtering produces a one-element keepaliveAgents array. The following length > 1 guard then skips that valid alternative and escalates to needs-human; determine whether the filtered set contains an agent other than the current one rather than requiring two available agents.
Useful? React with 👍 / 👎.
bdeee7d to
cde185e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/agents-auto-pilot.yml:
- Around line 3370-3404: Extract the duplicated ordered workflow-dispatch
fallback logic from the current block and the local
dispatchFirstAvailableWorkflow implementation into a shared helper under
.github/scripts/*.js. Update both github-script steps to import and reuse that
helper, preserving ordered attempts, 404-only fallback behavior, failure
collection, and the existing no-dispatch error semantics.
- Around line 3370-3404: Ensure the agent:auto label update occurs only after
the dispatch loop successfully sets dispatchedWorkflow. Move the unconditional
label-add logic associated with the handoff below the successful dispatch
confirmation, or remove the label when both workflow dispatch attempts fail, so
the outer catch cannot leave agent:auto blocking future retries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7b098871-9dde-44fd-a747-f822c7b62d2d
📒 Files selected for processing (2)
.github/workflows/agents-auto-pilot.ymltemplates/consumer-repo/.github/workflows/agents-auto-pilot.yml
| const { data: repoInfo } = await withRetry((client) => client.rest.repos.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| })); | ||
| let dispatchedWorkflow = ''; | ||
| const dispatchFailures = []; | ||
| for (const workflowId of [ | ||
| 'agents-81-gate-followups.yml', | ||
| 'agents-keepalive-loop.yml', | ||
| ]) { | ||
| try { | ||
| await withRetry((client) => client.rest.actions.createWorkflowDispatch({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| workflow_id: workflowId, | ||
| ref: repoInfo.default_branch || 'main', | ||
| inputs: { | ||
| pr_number: String(prNumber), | ||
| force_retry: 'true', | ||
| }, | ||
| })); | ||
| dispatchedWorkflow = workflowId; | ||
| break; | ||
| } catch (dispatchError) { | ||
| dispatchFailures.push( | ||
| `${workflowId}: ${dispatchError?.message || dispatchError}` | ||
| ); | ||
| if (dispatchError?.status !== 404) break; | ||
| } | ||
| } | ||
| if (!dispatchedWorkflow) { | ||
| throw new Error( | ||
| `No keepalive handoff workflow could be dispatched: ${dispatchFailures.join('; ')}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial
Duplicate "dispatch first available workflow with fallback" logic.
This ordered-dispatch-with-non-404-fallback loop mirrors dispatchFirstAvailableWorkflow already defined locally in the "Execute step - Create PR" step (around line 2322). Since each github-script step is an isolated execution, this can't currently be shared without extracting it into a .github/scripts/*.js helper. Consider factoring this into a shared script to avoid the two implementations drifting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/agents-auto-pilot.yml around lines 3370 - 3404, Extract
the duplicated ordered workflow-dispatch fallback logic from the current block
and the local dispatchFirstAvailableWorkflow implementation into a shared helper
under .github/scripts/*.js. Update both github-script steps to import and reuse
that helper, preserving ordered attempts, 404-only fallback behavior, failure
collection, and the existing no-dispatch error semantics.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
agent:auto is committed to the issue/PR before the dispatch is confirmed to succeed — a failed dispatch permanently disables this handoff.
The unconditional agent:auto label add (lines 3350-3369, unchanged context) happens before this new dispatch loop. If neither agents-81-gate-followups.yml nor agents-keepalive-loop.yml can be dispatched, the loop throws (line 3400-3404), the outer catch swallows it, and handedOff stays false — but agent:auto is now stuck on the issue (and possibly the PR). On any future run, the guard if (!hasAuto && hasAlternative) at line 3349 will see hasAuto === true and permanently skip this handoff path, even after the underlying dispatch problem (e.g. missing workflow, transient outage) is fixed and auto-pilot is resumed.
Reorder so the labels are only added after a workflow is successfully dispatched, or remove agent:auto on dispatch failure so future stall cycles can retry.
🔧 Proposed reordering
const hasAlternative = keepaliveAgents.some((agent) => agent !== currentAgent);
if (!hasAuto && hasAlternative) {
- // Add agent:auto to the issue AND the PR so the keepalive delegation
- // policy picks an alternative agent on its next round.
- await withRetry((client) => client.rest.issues.addLabels({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: issueNumber,
- labels: ['agent:auto'],
- }));
- if (prNumber) {
- try {
- await withRetry((client) => client.rest.issues.addLabels({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: parseInt(prNumber, 10),
- labels: ['agent:auto'],
- }));
- } catch (prLabelErr) {
- core.info(`(could not label PR #${prNumber} with agent:auto: ${prLabelErr?.message})`);
- }
- }
const { data: repoInfo } = await withRetry((client) => client.rest.repos.get({
owner: context.repo.owner,
repo: context.repo.repo,
}));
let dispatchedWorkflow = '';
const dispatchFailures = [];
for (const workflowId of [
'agents-81-gate-followups.yml',
'agents-keepalive-loop.yml',
]) {
try {
await withRetry((client) => client.rest.actions.createWorkflowDispatch({
owner: context.repo.owner,
repo: context.repo.repo,
workflow_id: workflowId,
ref: repoInfo.default_branch || 'main',
inputs: {
pr_number: String(prNumber),
force_retry: 'true',
},
}));
dispatchedWorkflow = workflowId;
break;
} catch (dispatchError) {
dispatchFailures.push(
`${workflowId}: ${dispatchError?.message || dispatchError}`
);
if (dispatchError?.status !== 404) break;
}
}
if (!dispatchedWorkflow) {
throw new Error(
`No keepalive handoff workflow could be dispatched: ${dispatchFailures.join('; ')}`
);
}
+ // Only commit the routing label switch once the handoff workflow
+ // has actually been dispatched, so a failed dispatch can be retried
+ // on the next stall cycle instead of permanently blocking on hasAuto.
+ await withRetry((client) => client.rest.issues.addLabels({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: issueNumber,
+ labels: ['agent:auto'],
+ }));
+ if (prNumber) {
+ try {
+ await withRetry((client) => client.rest.issues.addLabels({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: parseInt(prNumber, 10),
+ labels: ['agent:auto'],
+ }));
+ } catch (prLabelErr) {
+ core.info(`(could not label PR #${prNumber} with agent:auto: ${prLabelErr?.message})`);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: repoInfo } = await withRetry((client) => client.rest.repos.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| })); | |
| let dispatchedWorkflow = ''; | |
| const dispatchFailures = []; | |
| for (const workflowId of [ | |
| 'agents-81-gate-followups.yml', | |
| 'agents-keepalive-loop.yml', | |
| ]) { | |
| try { | |
| await withRetry((client) => client.rest.actions.createWorkflowDispatch({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| workflow_id: workflowId, | |
| ref: repoInfo.default_branch || 'main', | |
| inputs: { | |
| pr_number: String(prNumber), | |
| force_retry: 'true', | |
| }, | |
| })); | |
| dispatchedWorkflow = workflowId; | |
| break; | |
| } catch (dispatchError) { | |
| dispatchFailures.push( | |
| `${workflowId}: ${dispatchError?.message || dispatchError}` | |
| ); | |
| if (dispatchError?.status !== 404) break; | |
| } | |
| } | |
| if (!dispatchedWorkflow) { | |
| throw new Error( | |
| `No keepalive handoff workflow could be dispatched: ${dispatchFailures.join('; ')}` | |
| ); | |
| } | |
| const { data: repoInfo } = await withRetry((client) => client.rest.repos.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| })); | |
| let dispatchedWorkflow = ''; | |
| const dispatchFailures = []; | |
| for (const workflowId of [ | |
| 'agents-81-gate-followups.yml', | |
| 'agents-keepalive-loop.yml', | |
| ]) { | |
| try { | |
| await withRetry((client) => client.rest.actions.createWorkflowDispatch({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| workflow_id: workflowId, | |
| ref: repoInfo.default_branch || 'main', | |
| inputs: { | |
| pr_number: String(prNumber), | |
| force_retry: 'true', | |
| }, | |
| })); | |
| dispatchedWorkflow = workflowId; | |
| break; | |
| } catch (dispatchError) { | |
| dispatchFailures.push( | |
| `${workflowId}: ${dispatchError?.message || dispatchError}` | |
| ); | |
| if (dispatchError?.status !== 404) break; | |
| } | |
| } | |
| if (!dispatchedWorkflow) { | |
| throw new Error( | |
| `No keepalive handoff workflow could be dispatched: ${dispatchFailures.join('; ')}` | |
| ); | |
| } | |
| // Only commit the routing label switch once the handoff workflow | |
| // has actually been dispatched, so a failed dispatch can be retried | |
| // on the next stall cycle instead of permanently blocking on hasAuto. | |
| await withRetry((client) => client.rest.issues.addLabels({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: issueNumber, | |
| labels: ['agent:auto'], | |
| })); | |
| if (prNumber) { | |
| try { | |
| await withRetry((client) => client.rest.issues.addLabels({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: parseInt(prNumber, 10), | |
| labels: ['agent:auto'], | |
| })); | |
| } catch (prLabelErr) { | |
| core.info(`(could not label PR #${prNumber} with agent:auto: ${prLabelErr?.message})`); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/agents-auto-pilot.yml around lines 3370 - 3404, Ensure the
agent:auto label update occurs only after the dispatch loop successfully sets
dispatchedWorkflow. Move the unconditional label-add logic associated with the
handoff below the successful dispatch confirmation, or remove the label when
both workflow dispatch attempts fail, so the outer catch cannot leave agent:auto
blocking future retries.
Summary
agent:autobefore belt rotation and provision tried-marker labelsValidation
node --test .github/scripts/__tests__/agent-stall-rotation.test.jsactionlint .github/workflows/agents-auto-pilot.yml templates/consumer-repo/.github/workflows/agents-auto-pilot.ymlThe focused Python workflow tests could not collect in the isolated clone because pytest used an interpreter without PyYAML.
Summary by CodeRabbit