[DEV-106] Automatic failover across multiple agent harnesses when usage limits are hit - #98
Open
danii1 wants to merge 1 commit into
Open
[DEV-106] Automatic failover across multiple agent harnesses when usage limits are hit#98danii1 wants to merge 1 commit into
danii1 wants to merge 1 commit into
Conversation
…rnesses when usage limits are hit
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task: DEV-106
Summary: Automatic failover across multiple agent harnesses when usage limits are hit
Implementation Details
I'll start by exploring the existing codebase to understand the structures mentioned in the task.
Now let me look at supporting files: agent-model, run-recorder, and the docs/env example.
Now let me check a few more call sites and existing test patterns.
I now have a full picture. Let me set up a task list and implement.
Now creating the harness-chain module in agent-harness:
Let me refine that module — the fallback path is convoluted:
Now update
resolveHarnessin resolver.ts to handle comma-separatedAGENT_HARNESS:parseHarnessListcanonicalizes aliases, which would break the deprecation-warning path inresolveHarness(it checks the raw name). Let me use the raw first token there instead:There's now a circular import (resolver ↔ harness-chain), and
import/no-cycleis an error in this repo. Let me moveDEFAULT_HARNESS_NAMEintoregistry.ts:Now export the new module from the agent-harness index:
Now the failover state machine:
Now add active-harness persistence to
webhook-queue.ts:Before wiring the server, let me check how the webhook integration test drives this module:
Let me check how
processReviewAsync's internal catch interacts withUsageLimitError:The UsageLimitError thrown in
processReviewAsyncappears swallowed by its own catch. Let me check git history to understand:Confirmed: the internal catch swallows
UsageLimitError, making the wrapper's pause logic unreachable — I'll need to re-throw it there for failover to work. Now let me wire the server. First, imports and module state:Now replace the module state and the pause/resume functions:
Edge case:
resolveHarnessChaincan return empty entries when every name is unknown, which would makeHarnessFailoverthrow an unhelpful error. An all-unknown chain should throw the familiar config error instead:Now update the catch blocks and spawn sites:
Now the re-throw in
processReviewAsync's catch so the wrapper can fail over:Now update the startup logic in
startWebhookServer:Update the health endpoint to surface failover state:
Now fix the run-record harness name in review-polling-acquirer:
Dependencies aren't installed in this worktree. Let me install:
Typecheck passes. Now let me check harness
defaultPathvalues for tests, and the webhook-queue test pattern:Now write the harness-chain tests:
Two issues: the all-not-installed fallback still applies the installability drop, and the alias deprecation warning is never emitted by the chain resolver. Let me fix both:
All 23 pass. Now the failover tests:
Two real issues: my test helper captures
activeby value (closure bug), andrestoredoesn't clear stale persisted rows. Fixing the helper first:Now fix
restoreto clear stale/expired persisted rows:All pass. Now run the full suites for both packages:
All green. Now docs. Let me view the relevant sections:
Now the .env.example files:
Docs done. Let me now review the full webhook-server diff for correctness:
One stale comment references
initializeFailover(I named itensureFailover). Also I need to verify the pruning semantics inwindows()/earliestResetMs()— the timer callback iterateswindows()and expects expired entries to still be there:windows()prunes expired entries, but the timer callback needs to see them to log the failback. Let me switch to pure time-comparison semantics (no pruning):Fix the stale comment in webhook-server:
The only lint error is in my new test file (inline type specifier). Fixing:
Lint exits 0 now. Let me check that my files add no new warnings and re-run everything from root:
Let me verify run-record harness metadata in the CLI flow, and do a final review of the new modules:
The CLI task pipeline doesn't record the harness in run records yet — the AC asks for this. Let me find how the workflow resolves its harness:
All pass. Final review of the complete new modules:
Two small cleanups — a misleading doc phrase in
restoreand a redundant re-read inreportUsageLimit:The startup block is solid. Final full verification across the monorepo:
DEV-106 is implemented and verified (typecheck, lint, format, and all package test suites pass — 40 new tests added).
What was built
@devintern/agent-harnesssrc/harness-chain.ts(new):parseHarnessList(comma-split, trim, alias resolution, canonical dedupe,claude-codedefault) andresolveHarnessChain(registry validation, per-harness CLI path resolution, installability checks with injectable predicate). Unknown/not-installed entries become warnings + are skipped; an all-unknown chain throws the familiar config error; an all-not-installed chain is kept so the spawn surfaces the real "CLI not found" error.AGENT_CLI_PATHapplies to the primary entry only so a stale global override can't leak onto a fallback.src/resolver.ts:resolveHarness()now takes the first entry of a comma-separatedAGENT_HARNESS, keeping one-shot/review-helper flows exactly as today.DEFAULT_HARNESS_NAMEmoved toregistry.ts(avoids animport/no-cycleerror).@getdevintern/codeworker mode (webhook server)src/lib/harness-failover.ts(new): deterministic state machine — the active harness is always the highest-priority non-limited entry.reportUsageLimitextends windows (furthest reset wins), switches/fails-over, or reportsexhausted;windowElapsedclears windows and logs failback to the primary; persistence/clock/log hooks are injected.src/webhook-server.ts: startup resolves the chain, warns + skips bad entries, logsAgent harness: a → b (failover enabled), and recovers persisted windows/active harness (stale state dropped with warnings).handleUsageLimitfails over and keeps processing; the queue only pauses when all harnesses are limited, resuming at the earliest reset. Failback timers re-arm automatically. All spawn sites useresolveActiveHarness()so<HARNESS>_CLI_PATHresolves per active harness;/healthreports active harness, chain, and open windows. Also fixed a latent bug:processReviewAsync's catch swallowedUsageLimitError, making queue pause/failover unreachable.src/lib/webhook-queue.ts:getAllRateLimits,setActiveHarness/getActiveHarness/clearActiveHarness(persisted per-harness inwebhook_meta, surviving restarts).harnessfor task, pr_mention, and base-sync runs; the review acquirer parses the primary name instead of storing the raw list.Docs:
docs/code/configuration.mdgains a "Failover across multiple harnesses" section (syntax, priority semantics, failover/failback, edge cases, env overrides); both.env.examplefiles document the list form.Tests:
packages/agent-harness/tests/harness-chain.test.ts(23) andpackages/code/tests/harness-failover.test.ts(17, including restart round-trips through a real SQLite queue DB).This PR was automatically created by @devintern/code