Bounded agent stall rotation before needs-human (auto-pilot) - #2814
Conversation
Auto-pilot previously terminated a stuck issue at needs-human as soon as the FIRST agent hit maxStallRetries. This adds a bounded, registry-driven rotation to a different eligible agent before escalating to a human, so a stall on one agent no longer strands the issue when another could succeed. - New pure helper .github/scripts/agent_stall_rotation.js (decideStallRotation): registry- and capability-driven, never hard-codes an agent list; the stalled agent is recorded via an `agents:tried-<agent>` label so each rotation shrinks the candidate set and stops once all eligible agents are tried (bounded). Unit-tested (node --test), incl. bounded-exhaustion and no-eligible-agents guards. - Belt lane (create-PR: no branch / no commits) has no PR/keepalive history, so it uses decideStallRotation (capability `belt`) + re-dispatches the belt via agents-71-codex-belt-dispatcher.yml with the next agent_key. - Monitor-PR lane (a PR exists) reuses the EXISTING registry-driven delegation policy (agent_delegation_policy.js) instead of rebuilding rotation: it adds `agent:auto` so decideNextAgent rotates across the fuller keepalive-capable set (codex/claude/cursor/gemini). Escalates if agent:auto is already present. - Fail-safe: needs-human is withheld ONLY if a rotation is successfully actioned; any error or "no untried eligible agent" falls through to the pre-existing escalation, so worst case == prior behavior. - Rotation comments carry the "Auto-pilot step" marker so countConsecutiveWaits resets and each rotated agent gets its own full maxStallRetries budget. Applied identically to root and consumer-template workflows; helper added to sync-manifest; docs/keepalive/Agents.md updated. actionlint clean; all 1295 .github/scripts tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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. |
📝 WalkthroughWalkthroughAdds registry-driven stall rotation for belt workers and PR keepalive monitoring, tracks tried agents with labels, updates consumer synchronization, adds unit tests, and documents the belt and keepalive routing paths. ChangesAuto-pilot stall rotation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AutoPilot as auto-pilot workflow
participant Rotation as agent_stall_rotation.js
participant GitHub as GitHub labels and workflows
participant Keepalive as keepalive delegation policy
AutoPilot->>Rotation: evaluate stalled belt agent
Rotation-->>AutoPilot: next eligible agent or escalation
AutoPilot->>GitHub: update labels and redispatch belt workflow
AutoPilot->>Keepalive: add agent:auto on monitor-pr stall
Keepalive-->>AutoPilot: delegated keepalive handling
AutoPilot->>GitHub: apply needs-human on failed handoff
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Workflow source neededPR #2814 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: 9711965
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: 8916ad94dd
ℹ️ 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.issues.addLabels({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: issueNumber, | ||
| labels: ['agent:auto'], |
There was a problem hiding this comment.
In the normal monitor-pr path the PR has been using an explicit agent:<name> label, so keepalive has not persisted delegation state (keepalive_loop.js only writes current_agent when routing mode is already auto). Adding only agent:auto here makes decideNextAgent take its initial-selection branch and choose the registry default, so a stalled Codex PR is handed back to Codex while auto-pilot stops and withholds the prior needs-human escalation. This needs to seed the previous agent/tried state or directly switch to a different keepalive-capable agent before setting handedOff=true.
Useful? React with 👍 / 👎.
| const decision = decideStallRotation({ | ||
| registry: rotationRegistry, | ||
| labels: rotLabels, | ||
| capability: 'belt', | ||
| }); |
There was a problem hiding this comment.
When a consumer has only the current agent's credentials, this call still treats every registry belt-capable agent as eligible because no secrets map is supplied, so a Codex stall can switch to Claude even though the subsequent runner cannot authenticate; the issue then burns another full stall budget instead of falling through to needs-human. The helper already supports required_secrets filtering, so the workflow should pass the same secret-availability booleans before deciding to rotate.
AGENTS.md reference: AGENTS.md:L47-L48
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/scripts/agent_stall_rotation.js:
- Around line 47-57: The production rotation eligibility path must not treat
omitted secret availability as eligible. Update
.github/scripts/agent_stall_rotation.js lines 47-57 and
templates/consumer-repo/.github/scripts/agent_stall_rotation.js lines 47-57 to
require an explicit secret-presence map from decideStallRotation and
eligibleAgents callers, or fail closed when required-secret availability is
absent, preserving escalation for agents without configured credentials.
In @.github/workflows/agents-auto-pilot.yml:
- Around line 2670-2684: The “Branch was not created” escalation comment is
over-indented, causing its Markdown body to render as a code block. In both
.github/workflows/agents-auto-pilot.yml lines 2670-2684 and
templates/consumer-repo/.github/workflows/agents-auto-pilot.yml lines 2670-2684,
dedent the template-literal content to match the 12-space indentation used by
the sibling “Belt worker failed to generate commits” comment.
- Around line 2544-2621: Remove the unused withRetry, context, and core
properties from the first tryBeltStallRotation call site, keeping only the
arguments consumed by tryBeltStallRotation: issueNumber, stallCount, and
warnMsg. Preserve the helper’s closure-based access and align this invocation
with the second call site.
🪄 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: 02cb6c67-775f-4fc2-a851-1fa5cde9f592
📒 Files selected for processing (7)
.github/scripts/__tests__/agent-stall-rotation.test.js.github/scripts/agent_stall_rotation.js.github/sync-manifest.yml.github/workflows/agents-auto-pilot.ymldocs/keepalive/Agents.mdtemplates/consumer-repo/.github/scripts/agent_stall_rotation.jstemplates/consumer-repo/.github/workflows/agents-auto-pilot.yml
| if (secrets) { | ||
| const required = config.required_secrets || []; | ||
| const mode = config.required_secrets_mode || 'all'; | ||
| const present = | ||
| mode === 'any' | ||
| ? required.length === 0 || required.some((secret) => Boolean(secrets[secret])) | ||
| : required.every((secret) => Boolean(secrets[secret])); | ||
| if (!present) { | ||
| continue; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not bypass credential eligibility in production rotation.
The supplied consumer workflow calls decideStallRotation and eligibleAgents without secrets, so this branch treats every capability-enabled agent as available. A stalled configured agent can therefore rotate to an agent with no configured credential instead of escalating. Pass an explicit secret-presence map at both workflow call sites, or make this path fail closed when availability is required.
.github/scripts/agent_stall_rotation.js#L47-L57: ensure production callers cannot omit required-secret availability.templates/consumer-repo/.github/scripts/agent_stall_rotation.js#L47-L57: apply the same contract in the synced consumer copy.
📍 Affects 2 files
.github/scripts/agent_stall_rotation.js#L47-L57(this comment)templates/consumer-repo/.github/scripts/agent_stall_rotation.js#L47-L57
🤖 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/scripts/agent_stall_rotation.js around lines 47 - 57, The production
rotation eligibility path must not treat omitted secret availability as
eligible. Update .github/scripts/agent_stall_rotation.js lines 47-57 and
templates/consumer-repo/.github/scripts/agent_stall_rotation.js lines 47-57 to
require an explicit secret-presence map from decideStallRotation and
eligibleAgents callers, or fail closed when required-secret availability is
absent, preserving escalation for agents without configured credentials.
| async function tryBeltStallRotation({ issueNumber, stallCount, warnMsg }) { | ||
| try { | ||
| const { decideStallRotation } = require('./.github/scripts/agent_stall_rotation.js'); | ||
| const { loadAgentRegistry } = require('./.github/scripts/agent_registry.js'); | ||
| const rotationRegistry = loadAgentRegistry(); | ||
| const { data: rotIssue } = await withRetry((client) => client.rest.issues.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: issueNumber, | ||
| })); | ||
| const rotLabels = (rotIssue.labels || []) | ||
| .map((l) => (typeof l === 'string' ? l : l && l.name)) | ||
| .filter(Boolean); | ||
| const decision = decideStallRotation({ | ||
| registry: rotationRegistry, | ||
| labels: rotLabels, | ||
| capability: 'belt', | ||
| }); | ||
| if (!decision.rotate) { | ||
| core.info(`Belt stall rotation unavailable (${decision.reason}); escalating`); | ||
| return false; | ||
| } | ||
| // Record the stalled agent as tried and switch the explicit agent label. | ||
| if (decision.currentAgent) { | ||
| try { | ||
| await withRetry((client) => client.rest.issues.removeLabel({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: issueNumber, | ||
| name: `agent:${decision.currentAgent}`, | ||
| })); | ||
| } catch (labelErr) { | ||
| core.info(`(agent:${decision.currentAgent} label not present: ${labelErr?.message})`); | ||
| } | ||
| } | ||
| await withRetry((client) => client.rest.issues.addLabels({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: issueNumber, | ||
| labels: [`agent:${decision.nextAgent}`, decision.triedMarker].filter(Boolean), | ||
| })); | ||
| // Re-dispatch the belt for the NEW agent (same mechanism as backoff). | ||
| const { data: repoInfo } = await withRetry((client) => client.rest.repos.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| })); | ||
| const dispatchRef = repoInfo.default_branch || 'main'; | ||
| await withRetry((client) => client.rest.actions.createWorkflowDispatch({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| workflow_id: 'agents-71-codex-belt-dispatcher.yml', | ||
| ref: dispatchRef, | ||
| inputs: { | ||
| agent_key: decision.nextAgent, | ||
| force_issue: String(issueNumber), | ||
| dry_run: 'false', | ||
| }, | ||
| })); | ||
| // NOTE: the leading "🤖 **Auto-pilot step**" marker is deliberate — it | ||
| // makes countConsecutiveWaits() treat this comment as a boundary and | ||
| // reset the stall counter, so the newly-rotated agent gets its own full | ||
| // maxStallRetries budget instead of inheriting the prior agent's count. | ||
| await withRetry((client) => client.rest.issues.createComment({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: issueNumber, | ||
| body: `🤖 **Auto-pilot step ${typeof stepCount !== 'undefined' ? stepCount : '?'}**: rotating agent after stall\n\n` + | ||
| `\`${decision.currentAgent || 'current agent'}\` stalled after ${stallCount} attempts (${warnMsg}). ` + | ||
| `Switching to \`${decision.nextAgent}\` and re-dispatching the belt.\n\n` + | ||
| `\`needs-human\` is withheld until every eligible agent has been tried.`, | ||
| })); | ||
| core.info(`Belt stall rotation: ${decision.currentAgent || '(none)'} -> ${decision.nextAgent}`); | ||
| return true; | ||
| } catch (rotationErr) { | ||
| core.warning(`Belt stall rotation failed, falling back to needs-human: ${rotationErr?.message}`); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Rotation helper is sound and fail-safe. Decision is pure/registry-driven, tried-marker bounding is correct, and any error path returns false to preserve the existing needs-human escalation.
Minor note: the first call site (Line 2654) passes withRetry, context, core into the argument object, but the helper only destructures { issueNumber, stallCount, warnMsg } and reads withRetry/context/core from the closure. The extra args are silently ignored and diverge from the second call site (Line 2886). Consider dropping them to avoid implying a wider contract.
🤖 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 2544 - 2621, Remove the
unused withRetry, context, and core properties from the first
tryBeltStallRotation call site, keeping only the arguments consumed by
tryBeltStallRotation: issueNumber, stallCount, and warnMsg. Preserve the
helper’s closure-based access and align this invocation with the second call
site.
| body: `## ⚠️ Auto-Pilot Stalled | ||
|
|
||
| **Reason:** Branch was not created after ${stallCount} attempts. | ||
| **Reason:** Branch was not created after ${stallCount} attempts. | ||
|
|
||
| Expected branch: \`${branchName}\` | ||
| Expected branch: \`${branchName}\` | ||
|
|
||
| **Possible causes:** | ||
| - Agent failed before branch creation | ||
| - Workflow dispatch failed silently | ||
| - Permissions or token issues | ||
| **Possible causes:** | ||
| - Agent failed before branch creation | ||
| - Workflow dispatch failed silently | ||
| - Permissions or token issues | ||
| - Every eligible agent has already been tried | ||
|
|
||
| **To resume:** Remove \`agents:auto-pilot-pause\` and | ||
| \`needs-human\` labels, then re-add \`agents:auto-pilot\`.` | ||
| })); | ||
| **To resume:** Remove \`agents:auto-pilot-pause\` and | ||
| \`needs-human\` labels, then re-add \`agents:auto-pilot\`.` | ||
| })); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Dedent the "branch not created" escalation comment to match the sibling block. In both workflows the new if (!rotated) wrapper caused this comment body to be indented ~22 spaces, while the parallel "Belt worker failed to generate commits" comment (Lines 2900-2918) and all other comment heredocs stay at 12 spaces. The extra leading whitespace changes the rendered notice (Markdown renders 4+ leading spaces as a code block), producing an inconsistent human-escalation message.
.github/workflows/agents-auto-pilot.yml#L2670-L2684: dedent the template-literal content to 12 spaces to match Line 2900's block.templates/consumer-repo/.github/workflows/agents-auto-pilot.yml#L2670-L2684: apply the identical dedent.
📍 Affects 2 files
.github/workflows/agents-auto-pilot.yml#L2670-L2684(this comment)templates/consumer-repo/.github/workflows/agents-auto-pilot.yml#L2670-L2684
🤖 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 2670 - 2684, The
“Branch was not created” escalation comment is over-indented, causing its
Markdown body to render as a code block. In both
.github/workflows/agents-auto-pilot.yml lines 2670-2684 and
templates/consumer-repo/.github/workflows/agents-auto-pilot.yml lines 2670-2684,
dedent the template-literal content to match the 12-space indentation used by
the sibling “Belt worker failed to generate commits” comment.
What & why
Auto-pilot terminated a stuck issue at
needs-humanas soon as the first agent hitmaxStallRetries. This adds a bounded, registry-driven rotation to a different eligible agent before escalating to a human — directly addressing the failure mode where every correctness fix in a repo stalled on one agent and dead-ended atneeds-human(observed across the Pension-Data audit backlog).Design (dedup-aware)
agent_stall_rotation.js→decideStallRotation({capability:'belt'})picks the next untried belt-capable agent, swaps theagent:<name>label, re-dispatchesagents-71-codex-belt-dispatcher.ymlwith the newagent_key.agent_delegation_policy.js) rather than rebuilding rotation — addsagent:autosodecideNextAgentrotates across the fuller keepalive set (codex/claude/cursor/gemini). Escalates ifagent:autoalready present.agents:tried-<agent>label; each rotation shrinks the candidate set; rotation stops (→ existingneeds-human) once every eligible agent is tried.belt/pr_keepalivecapable.needs-humanis withheld only if a rotation is successfully actioned. Any error, or "no untried eligible agent left", falls through to the pre-existing escalation — worst case is identical to prior behavior.Auto-pilot stepmarker socountConsecutiveWaitsresets and each rotated agent gets its own fullmaxStallRetriesbudget.Changes
.github/scripts/agent_stall_rotation.js(pure decision) +__tests__/agent-stall-rotation.test.js(8 tests: capability gating, secrets, belt exhaustion→needs-human, fuller keepalive set, deliberate-break guard, default-agent fallback).agents-auto-pilot.yml(identical)..github/sync-manifest.yml;docs/keepalive/Agents.mdupdated.Validation done locally
node --test .github/scripts/__tests__/*.test.js→ 1295 pass / 0 fail.actionlintclean on both workflows; all 14 inlinegithub-scriptblocks passnode --check(async-wrapped).Local checks prove the change doesn't break and is fail-safe, but they do not exercise the live rotation end-to-end (GitHub Actions
workflow_dispatchre-dispatch, label events, gate re-entry cannot run locally). Recommend validating on a deliberate-break test issue in one repo before un-drafting + merging + fleet sync. Fleet blast radius: syncs to all 12 consumer repos.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation