fix(ai): retry a transient provider 502/503 instead of failing the turn

This commit is contained in:
John Smith
2026-09-24 05:37:42 -04:00
parent 600d472d29
commit cbba33b651
4 changed files with 165 additions and 7 deletions
+9 -3
View File
@@ -7,7 +7,7 @@
"asset-curator",
"qa-playtester"
],
"last_cycle": 259,
"last_cycle": 260,
"completed_targets": [
{
"cycle": 75,
@@ -1528,10 +1528,15 @@
"target": "Phase 5: verify the manager-role fix with a fresh @feedback-loop run (no code change)",
"metric_delta": "No code change. Re-ran @feedback-loop (hosted proxy, key via the localhost helper + Preferences form, Empty 1280x720 project pre-created) to validate cycle 258. Result: the top-level orchestrator made 4 spawns (designer x2, developer x2) and 0 direct project edits over 53 messages, with the plan tracking the model's own ids (task_design done, task_build done, task_economy in_progress, task_test pending); its reasoning explicitly acknowledged the restricted toolset. Pre-fix Run 5 made 15 manager mutations. Recorded as Run 6 in the skill's runs/RUNS.md; screenshot /tmp/opencode/cycle259-manager-fixed.png. One child developer errored (28 msgs) - sub-agent failure recovery is the next thing to watch.",
"commit": "docs(autonomous): verify the manager-role fix end-to-end"
},
{
"cycle": 260,
"target": "Tier 2 resilience: retry a transient provider 502/503 instead of failing the agent turn",
"metric_delta": "feedback-loop Run 6's developer sub-agent died on a provider 503 - a transient 'endpoint unavailable' that failed the whole turn and wasted the agent's work (sub-agent failure recovery then spawned a replacement, but the provider blip was the root cause). Fixed: sendChatCompletion now attaches the HTTP status to the thrown provider error (providerStatus), and the three turn paths (create/continue/sub-agent) run through runTurnWithProviderRetry - 408/429/502/503/504 are retried up to 2 times with linear backoff (1.5s/3s), never on abort; a bare 500 and all 4xx are NOT retried (preserving the existing 'no 500 retry' contract, whose spec still passes). +3 specs (503 then success => 2 posts, ready; 400 => 1 post, error; retryable classification). Tests 1719 AI-surface / 83 suites. Live-verified with a purpose-built flaky provider (503 then 200): request ended ready, console logged the retry, provider log showed call 1 503 -> call 2 200. Build green; ESLint/Prettier/divergence clean.",
"commit": "fix(ai): retry a transient provider 502/503 instead of failing the turn"
}
],
"backlog": [
"Watch sub-agent failure recovery in a real run: Run 6 had a developer sub-agent end in 'error' (28 msgs); confirm the manager spawns a follow-up or reports it, and that the final report reaches the parent.",
"Tier 1 orchestration (follow-up to cycle 250): the nudge resumes a stalled manager, but the manager prompt itself could be tightened and a stronger local model still delegates more reliably; consider measuring the nudge's real-model effect with the prompt-city benchmark.",
"Harness: the committed scripts/dev/mock-ai-provider.js lets @feedback-loop run offline; use it to script deterministic tool-call sequences and assert the studio loop without a real model.",
"Watch for the ubuntu-latest -> Ubuntu 26 runner migration notice (dated 2026-10-19); workflows are restored from local master by upstream-sync.",
@@ -1748,6 +1753,7 @@
"Audit a tool's OpenAI schema against its implementation/specs whenever an agent reports a tool 'does nothing': change_behavior_property advertised flat args while reading changed_properties, so every schema-conformant call was a silent no-op. Sibling tools' schemas are the quickest reference for the expected shape.",
"Generalizable harness check: an editor tool's OpenAI schema must advertise every top-level arg its implementation reads (a mismatch makes the capability unreachable for a schema-conforming model). Audit by mapping export tool names to their const ranges and comparing extractArrayProperty/extractObjectProperty(args, 'x') reads against the schema properties.",
"A role-based agent must get BOTH its role prompt and its role tool subset at the top level too, not only for sub-agents. The BYOK orchestrator ran with every tool plus a generic prompt, so the 'manager' edited the project directly (feedback-loop Run 5); resolveStudioRoleId now maps orchestrator mode to the manager role while a stale role id still fails closed.",
"A fresh @feedback-loop run is the right validation for an orchestration fix: the deterministic spec pinned the tool subset, but only the real run proved the manager delegates (4 spawns, 0 direct edits) instead of building the project itself."
"A fresh @feedback-loop run is the right validation for an orchestration fix: the deterministic spec pinned the tool subset, but only the real run proved the manager delegates (4 spawns, 0 direct edits) instead of building the project itself.",
"Sub-agent failure recovery works (feedback-loop Run 6): an errored child is treated as finished, its failed report is delivered to the manager, and the manager spawned a replacement. The underlying 503 is now retried client-side, so a transient provider blip no longer kills a sub-agent turn. Keep 500 non-retryable (a genuine provider error should surface as-is)."
]
}
+75 -4
View File
@@ -3114,6 +3114,64 @@ const NETWORK_RETRY_DELAY_MS = 500;
const delay = (ms: number): Promise<void> =>
new Promise(resolve => setTimeout(resolve, ms));
// A provider can return a transient 429/5xx ("endpoint unavailable, reset after
// 6s") that succeeds seconds later. Without a retry the whole agent turn fails
// - which for a sub-agent wastes its work (feedback-loop Run 6: a developer died
// on a 503). Bounded and backed off, so a persistent outage still fails.
const PROVIDER_RETRY_DELAY_MS = 1500;
const MAX_PROVIDER_RETRIES = 2;
/** The HTTP status of a thrown provider error, or null. */
export const getProviderErrorStatus = (error: any): number | null => {
const status = error && (error: any).providerStatus;
return typeof status === 'number' ? status : null;
};
/**
* Whether a provider error is worth retrying: rate limiting (429), request
* timeout (408) or a gateway/server-availability failure (502/503/504). A 4xx
* client error and a bare 500 are NOT retried: 500 is deliberately left
* untouched so a genuine provider error reads exactly as the server sent it
* (see the status-hint comment in `sendChatCompletion`).
*/
export const isRetryableProviderError = (error: any): boolean => {
const status = getProviderErrorStatus(error);
if (status === null) return false;
return status === 408 || status === 429 || (status >= 502 && status <= 504);
};
/**
* Run a turn, retrying a transient provider error (429/5xx) with backoff.
* Aborted turns are never retried.
*/
const runTurnWithProviderRetry = async (
sendTurn: () => Promise<any>,
abortSignal: { aborted: boolean }
): Promise<any> => {
let attempt = 0;
for (;;) {
try {
return await sendTurn();
} catch (error) {
if (
abortSignal.aborted ||
!isRetryableProviderError(error) ||
attempt >= MAX_PROVIDER_RETRIES
) {
throw error;
}
attempt++;
const delayMs = PROVIDER_RETRY_DELAY_MS * attempt;
console.warn(
`[CustomAIClient] Transient provider error (${getProviderErrorStatus(
error
)}); retrying in ${delayMs}ms (attempt ${attempt}/${MAX_PROVIDER_RETRIES}).`
);
await delay(delayMs);
}
}
};
/**
* Whether an error is a transport-level failure (no HTTP response): the server
* is unreachable, the connection was reset, DNS failed, etc. A provider error
@@ -3798,11 +3856,15 @@ export const sendChatCompletion = async ({
: status === 502 || status === 503
? ' (The provider is temporarily unavailable: retry in a moment; if it persists, check the provider status or your base URL.)'
: '';
throw new Error(
const providerError = new Error(
`AI Provider Error (${status}): ${errorMsg}${authHint}${statusHint}${getErrorHint(
error
)}`
);
// Keep the HTTP status on the error so a caller can retry a transient
// provider failure (429/5xx) instead of failing the whole agent turn.
(providerError: any).providerStatus = status;
throw providerError;
}
// No response at all (server down, network split, DNS): add the local
// reachability/model hints when they match. The thrown value may be
@@ -4322,7 +4384,10 @@ export const customCreateAiRequest = async ({
let assistantResponse;
try {
try {
assistantResponse = await sendTurn();
assistantResponse = await runTurnWithProviderRetry(
sendTurn,
abortController.signal
);
} catch (error) {
// The first turn embeds the project structure, so it is the most likely
// to overflow: retry once at a halved budget before failing the create.
@@ -4582,7 +4647,10 @@ export const customAddMessageToAiRequest = async ({
let assistantResponse;
try {
try {
assistantResponse = await sendTurn();
assistantResponse = await runTurnWithProviderRetry(
sendTurn,
abortController.signal
);
} catch (error) {
// A context-overflow 400 means the request still exceeded the window
// even after the standard trim (an unknown model family defaulting too
@@ -4868,7 +4936,10 @@ export const customCreateSubAgentAiRequest = async ({
let assistantResponse;
try {
try {
assistantResponse = await sendTurn();
assistantResponse = await runTurnWithProviderRetry(
sendTurn,
abortController.signal
);
} catch (error) {
// Sub-agents receive the project JSON too, so they can overflow the
// same way: one retry at a halved budget before failing the child.
+71
View File
@@ -44,6 +44,8 @@ import {
customGetAiRequestProviderTelemetry,
isContextOverflowError,
isNetworkLevelError,
isRetryableProviderError,
getProviderErrorStatus,
estimateTokens,
estimateToolsTokens,
getMessageBudget,
@@ -1823,6 +1825,75 @@ describe('CustomAIClient', () => {
expect(isNetworkLevelError({ message: 'shuffle failure' })).toBe(false);
});
it('retries a transient provider 503 and succeeds', async () => {
// feedback-loop Run 6: a 503 ("endpoint unavailable") killed a developer
// sub-agent's turn outright. A transient server error must be retried.
setCustomEndpointConfig({
enabled: true,
baseUrl: 'http://localhost:11434/v1',
apiKey: '',
model: 'llama3.2',
temperature: 0.7,
});
// $FlowFixMe
axios.post.mockRejectedValueOnce({
response: { status: 503, data: { error: { message: 'unavailable' } } },
});
// $FlowFixMe
axios.post.mockResolvedValueOnce({
status: 200,
data: { choices: [{ message: { role: 'assistant', content: 'ok' } }] },
});
const result = await customCreateAiRequest({
userRequest: 'start',
mode: 'chat',
});
expect(result.status).not.toBe('error');
expect(axios.post).toHaveBeenCalledTimes(2);
});
it('does not retry a real client error (400)', async () => {
setCustomEndpointConfig({
enabled: true,
baseUrl: 'http://localhost:11434/v1',
apiKey: '',
model: 'llama3.2',
temperature: 0.7,
});
// $FlowFixMe
axios.post.mockRejectedValue({
response: { status: 400, data: { error: { message: 'bad request' } } },
});
const result = await customCreateAiRequest({
userRequest: 'start',
mode: 'chat',
});
expect(result.status).toBe('error');
expect(axios.post).toHaveBeenCalledTimes(1);
});
it('classifies which provider errors are retryable', () => {
const providerError = (status: number) => {
const error = new Error('provider');
// $FlowFixMe: the client attaches the status for exactly this.
(error: any).providerStatus = status;
return error;
};
expect(getProviderErrorStatus(providerError(503))).toBe(503);
expect(getProviderErrorStatus(new Error('x'))).toBeNull();
expect(isRetryableProviderError(providerError(429))).toBe(true);
expect(isRetryableProviderError(providerError(408))).toBe(true);
expect(isRetryableProviderError(providerError(502))).toBe(true);
expect(isRetryableProviderError(providerError(503))).toBe(true);
expect(isRetryableProviderError(providerError(504))).toBe(true);
// 500 is a genuine server error and is left to the caller (see the
// existing "no 500 retry" behaviour), as are 4xx client errors.
expect(isRetryableProviderError(providerError(500))).toBe(false);
expect(isRetryableProviderError(providerError(400))).toBe(false);
expect(isRetryableProviderError(providerError(401))).toBe(false);
expect(isRetryableProviderError(new Error('network'))).toBe(false);
});
it('retries once with a smaller prompt when the window overflows', async () => {
setCustomEndpointConfig({
enabled: true,
+10
View File
@@ -98,6 +98,16 @@ work; `endpoint` is the AI endpoint under test for that cycle.
looping until the guard tripped (cycles 239-241)..
## Proven workflows
- Resilience: transient provider 503 is retried (cycle 260): feedback-loop Run 6's
developer sub-agent died on a provider 503 ("endpoint unavailable; reset after
6s") - a transient error that failed the whole turn. `sendChatCompletion` now
keeps the HTTP status on the thrown error, and each of the three turn paths
(create / continue / sub-agent) runs through `runTurnWithProviderRetry`: a
408/429/502/503/504 is retried up to 2 times with linear backoff, never on
abort; a bare 500 and 4xx are left as-is (the existing "no 500 retry" contract
preserved). Verified live with a purpose-built flaky provider: call 1 → 503,
call 2 → 200, the chat request ended `ready`, and the console logged
`Transient provider error (503); retrying in 1500ms (attempt 1/2)`.
- Manager-role fix verified end-to-end (cycle 259): a fresh `@feedback-loop` run
(Empty project pre-created; `prompt-city.md`) showed the top-level orchestrator
make **4 spawns (designer ×2, developer ×2) and 0 direct project edits** over