fix(FN-8004): retry ACP provider blips in auto-merge instead of parking failed (#2157)
## What happened
FN-8004's implementation work finished and passed review. The auto-merge
then failed with `Grok ACP turn failed: Internal error` — a ~20 second
provider blip — and the task was parked `status: "failed"` with 8 files
of complete, reviewed work stranded on its branch.
The park is the interesting part: `status: "failed"` is precisely what
tells recovery to stop. So a misclassification here isn't a missed
retry, it's **terminal**. Both recovery paths were disabled by the same
wrong verdict:
- `maybeRetryTransientMerge` (inline, 3 retries w/ backoff) — never
fired once (`mergeTransientRetryCount: 0`).
- `recoverTransientMergeFailures` (self-healing sweep, exists exactly to
rescue parked in-review tasks) — skipped it, gated on the same
classifier.
## Three defects fixed
**1. No AI-provider failure class existed.** The AI merge drives a real
LLM turn, but `classifyTransientMergeError` only modeled git/lease/spawn
faults. Adds `ai-provider-turn-failure`.
**2. ACP dropped the error detail.** `promptAcpSession` rethrew the SDK
error unchanged, discarding the JSON-RPC `code`/`data` — the only
evidence the fault was provider-side. ("Internal error" is just the
standard text for `-32603`.) It now preserves them, keeping the original
as `cause`:
```
Internal error (acp rpc code -32603, retryable)
```
Classification anchors on that envelope, **not** on the bare `"Internal
error"` — matching that unanchored would disguise genuine application
defects as retryable blips. Only provider-fault codes (`-32603`,
`-32000`..`-32003`) are retryable; caller-fault codes
(`-32600`..`-32602`) stay permanent, since retrying just repeats the
failing call.
**3. Sweep/inline asymmetry** (found while tracing; latent and
unreported). The inline gate accepted `isTransientError(msg) ||
classify(msg)`, but the sweep consulted **only** the classifier. So
`ECONNRESET` / `socket hang up` during a merge earned inline retries and
then went **invisible to the sweep** once parked — stranded forever. The
classifier now delegates to `isTransientError`, so both gates agree by
construction.
To keep that delegation from importing the detector's
`usage-limit-detector → logger` chain (the chain FN-5627 split the
classifier out to avoid, which would break
`notification-service.test.ts`'s partial `vi.mock`), the pure predicates
moved to the import-free leaf `transient-error-patterns.ts`, re-exported
from `transient-error-detector.ts`. All 13 exports preserved, verified
programmatically.
## Loosened budgets
Per request, so more self-heals. Both apply **only** to errors already
proven transient; the ceiling and
`merger:transient-failure-budget-exhausted` audit path remain.
| Budget | Before | After |
|---|---|---|
| `MAX_AUTO_MERGE_TRANSIENT_RETRIES` | 3 | 5 (backoff
5s/10s/20s/40s/80s) |
| `MAX_TRANSIENT_MERGE_RECOVERIES` | 2 | 5 |
The bump broke two suites that had hardcoded the old `3`. Rather than
swap in another magic number, both now derive the cap from the constant
so future tuning doesn't re-break them.
## Verification
- `pnpm test:gate` green · `pnpm lint` clean · engine + ACP typecheck
clean · `pnpm verify:fast` PASS (5/5)
- ACP plugin 230 tests green · Grok plugin 64 green · engine
transient/merge suites 136 green
- Regression tests assert the **invariant across every surface** (per
*Fix the Invariant, Not the Repro*), not just the reported Grok string:
both ACP runtime prefixes, all retryable/non-retryable rpc codes, both
SDK error shapes, network delegation, class-ordering, and negative cases
proving bare `"Internal error"` and real defects stay permanent.
- A test caught a genuine bug in my own code mid-review (nested-shape
message shadowing), now fixed.
- `notifier.test.ts > "awaiting approval"` fails — **confirmed
pre-existing on clean main**, unrelated.
## Note
FN-8004's own branch (`fusion/fn-8004`) is still unmerged and its work
looks complete. Once this lands, its merge should be retried separately.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/fn-8004-acp-transient-merge-classification.md
Normal file
7
.changeset/fn-8004-acp-transient-merge-classification.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Auto-merge now retries AI provider blips instead of permanently failing the task.
|
||||
category: fix
|
||||
dev: ACP provider faults (`promptAcpSession` now preserves the JSON-RPC code as `acp rpc code -32603`) classify as transient via a new `ai-provider-turn-failure` class. `classifyTransientMergeError` also delegates to `isTransientError`, so the self-healing sweep and the inline retry gate share one definition — previously network errors got inline retries but were invisible to the sweep once parked `failed`. Pure predicates moved to the import-free leaf `transient-error-patterns.ts` (re-exported from `transient-error-detector.ts`) to keep the logger chain out of the classifier per FN-5627. Transient budgets raised: `MAX_AUTO_MERGE_TRANSIENT_RETRIES` 3→5, `MAX_TRANSIENT_MERGE_RECOVERIES` 2→5.
|
||||
@@ -705,7 +705,10 @@ If loop recovery times out during compact-and-resume and the executor does not u
|
||||
- `recoverMissingWorktreeReviewFailures()` is a narrow failed-review recovery: only `status: "failed"` `in-review` tasks with the explicit session-start signature `Refusing to start coding agent in missing worktree:` (from `assertValidWorktreeSession()`) are requeued. Recovery clears stale session metadata (`worktree`, `branch`, `sessionFile`, transient failure state), preserves valid step progress/retry counters, logs the auto-recovery reason, and moves the task back to `todo` for a clean retry.
|
||||
- `recoverMergeableReviewTasks()` only re-enqueues truly eligible tasks; retry-exhausted review tasks are skipped to avoid re-enqueue/no-op loops that keep refreshing `updatedAt`.
|
||||
- `recoverAlreadyMergedReviewTasks()` auto-finalizes retry-exhausted `in-review` tasks when self-healing can prove their work already landed on the merge target. On this landed-content path it clears soft blockers (`paused`, stale `status: "failed"`, and residual `error`) before moving to `done`; true hard blockers (for example incomplete steps, awaiting-user-review, or failed pre-merge workflow steps) still park the task in stable `in-review/failed` state with a blocker error instead of entering an auto-finalize loop. Already-merged/tip recovery must prove task ownership before setting `mergeDetails.mergeConfirmed` or moving to `done`: accepted evidence is a matching `Fusion-Task-Id`, matching `Fusion-Task-Lineage`, a task-ID anchored conventional subject, or a patch-id/tree-equal fallback from the canonical `fusion/<task-id>` branch whose tip and candidate commit are not explicitly attributed to another task/lineage. Foreign task tips (for example an FN-7143 row pointing at an FN-7187 tip) are rejected in place with `[recovery] already-merged rejected ... reason=foreign-task-tip` and `task:auto-recover-already-merged-rejected` audit metadata instead of finalizing the wrong task.
|
||||
- `recoverTransientMergeFailures()` handles retry-exhausted `in-review` merge failures only when `classifyTransientMergeError()` returns a bounded transient class: `lease-handoff-target-not-queued`, `spurious-concurrent-advance-same-sha`, or `process-spawn-failure` (`spawn ENOTDIR`, `spawn … ENOENT`, or a clean-room path reported as `is not a working tree`). Recovery resets `mergeRetries`, clears transient `status`/`error`, increments `mergeDetails.transientRecoveryCount`, and requeues auto-merge so the next attempt recreates the AI-merge clean room. The budget stays capped by `MAX_TRANSIENT_MERGE_RECOVERIES`; exhausted tasks remain parked with the `merger:transient-failure-budget-exhausted` audit path so real structural failures cannot loop forever. FN-6278 makes this recovery mostly after-the-fact insurance for cwd spawn faults: the merge runner now preflights reuse integration roots and repairs/reacquires missing or de-registered task worktrees before the first git spawn, so a stale `task.worktree` should not consume the transient recovery budget by repeatedly producing `spawn git ENOENT`.
|
||||
- `recoverTransientMergeFailures()` handles retry-exhausted `in-review` merge failures only when `classifyTransientMergeError()` returns a bounded transient class: `lease-handoff-target-not-queued`, `spurious-concurrent-advance-same-sha`, `process-spawn-failure` (`spawn ENOTDIR`, `spawn … ENOENT`, or a clean-room path reported as `is not a working tree`), `ai-provider-turn-failure`, or `network-transport-failure`.
|
||||
- FN-8004 added the last two classes. `ai-provider-turn-failure` covers ACP-backed merge models (Grok/OMP/generic ACP) whose turn fails provider-side: `promptAcpSession` preserves the JSON-RPC code as `… (acp rpc code -32603, retryable)` and the classifier anchors on that envelope or the `<Runtime> ACP turn failed:` prefix. Anchoring is deliberate — the bare JSON-RPC text is `Internal error`, which must never match unanchored or it would disguise genuine application defects as retryable blips. Only provider-fault codes (`-32603`, `-32000`…`-32003`) are retryable; caller-fault codes (`-32600`…`-32602`) stay permanent because retrying repeats the failing call.
|
||||
- `network-transport-failure` is a delegation to `isTransientError()`, closing a real asymmetry: the inline retry gate (`ProjectEngine.maybeRetryTransientMerge`) accepted `isTransientError(msg) || classifyTransientMergeError(msg)`, while this sweep consulted only the classifier. Errors such as `ECONNRESET`/`socket hang up` therefore earned inline retries but became invisible to the sweep once parked `failed` — stranding them permanently. Both gates now share one definition by construction. To keep this delegation from importing the detector's `usage-limit-detector.js → logger.js` chain (the chain FN-5627 split the classifier out to avoid), the pure predicates live in the import-free leaf `transient-error-patterns.ts`, re-exported by `transient-error-detector.ts`.
|
||||
- Because `status:"failed"` is itself what suppresses both recovery paths, a misclassification here is not merely a missed retry — it is terminal. FN-8004's 20-second Grok blip permanently parked a task whose branch held complete, reviewed work. Recovery resets `mergeRetries`, clears transient `status`/`error`, increments `mergeDetails.transientRecoveryCount`, and requeues auto-merge so the next attempt recreates the AI-merge clean room. The budget stays capped by `MAX_TRANSIENT_MERGE_RECOVERIES`; exhausted tasks remain parked with the `merger:transient-failure-budget-exhausted` audit path so real structural failures cannot loop forever. FN-6278 makes this recovery mostly after-the-fact insurance for cwd spawn faults: the merge runner now preflights reuse integration roots and repairs/reacquires missing or de-registered task worktrees before the first git spawn, so a stale `task.worktree` should not consume the transient recovery budget by repeatedly producing `spawn git ENOENT`.
|
||||
- `reconcileTaskWorktreeMetadata()` (FN-4962) reconciles stale `task.worktree`/`task.branch` rows against authoritative `git worktree list --porcelain` branch mappings during startup recovery, periodic maintenance, and completion fan-out. The stage must run before `reclaim-stale-active-branches`: stale rows rebound to live `fusion/<id>` worktrees emit `task:auto-recover-worktree-metadata-rebound`; stale rows with no live branch mapping are nulled (`worktree=null`, `branch=null`, `baseCommitSha` unchanged) and emit `task:auto-recover-worktree-metadata-cleared`.
|
||||
- `recoverInProgressLimbo()` (FN-5219) is the safety net for stranded executor rows: reset/requeue paths must never leave a task in `in-progress` without a runnable execution context. After metadata reconcile, stale `in-progress` tasks with null branch, missing/cleared worktree metadata, no live executor claim, and all-pending steps are audited and moved back to `todo`.
|
||||
|
||||
|
||||
@@ -651,8 +651,13 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
});
|
||||
|
||||
it("parks direct merge when transient retry cap is exhausted", async () => {
|
||||
// FNXC:MergeReliability 2026-07-15-19:25 (FN-8004): seed AT the cap, read from the constant.
|
||||
// This previously hardcoded 3; raising the budget to 5 silently turned this into a
|
||||
// "retries once more" case. Deriving the seed keeps the invariant (park once the budget is
|
||||
// spent) under test regardless of how the budget is tuned.
|
||||
const atCap = ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES;
|
||||
const store = makeStore({
|
||||
tasks: [makeTask({ mergeTransientRetryCount: 3 }), makeTask({ mergeTransientRetryCount: 3 })],
|
||||
tasks: [makeTask({ mergeTransientRetryCount: atCap }), makeTask({ mergeTransientRetryCount: atCap })],
|
||||
});
|
||||
vi.mocked(runAiMerge).mockRejectedValueOnce(new Error("socket hang up"));
|
||||
|
||||
|
||||
@@ -234,7 +234,10 @@ describe("FN-5742 dual-observe merge seam", () => {
|
||||
getActiveMergingTask: vi.fn().mockReturnValue(null),
|
||||
} as any;
|
||||
|
||||
if ((ProjectEngine.prototype as any).isTransientMergeRetryExhausted.call({}, { mergeTransientRetryCount: 3 }, "socket hang up")) {
|
||||
// FNXC:MergeReliability 2026-07-15-19:25 (FN-8004): derive the exhausted seed from the
|
||||
// constant. Hardcoding 3 silently inverted this test's premise when the budget rose to 5.
|
||||
const exhaustedCount = ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES;
|
||||
if ((ProjectEngine.prototype as any).isTransientMergeRetryExhausted.call({}, { mergeTransientRetryCount: exhaustedCount }, "socket hang up")) {
|
||||
const record = store.getMergeRequestRecord("FN-MR");
|
||||
if (record.state === "running") {
|
||||
store.transitionMergeRequestState("FN-MR", "retrying", { attemptCount: record.attemptCount, lastError: "socket hang up" });
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
// FNXC:Reliability-ErrorClassification 2026-07-15-19:15 (FN-8004): the pure predicates moved to
|
||||
// the import-free leaf `transient-error-patterns.ts`; this module re-exports them. Importing via
|
||||
// BOTH paths here pins the re-export contract so existing callers keep working.
|
||||
import { isTransientError as isTransientErrorViaLeaf } from "../transient-error-patterns.js";
|
||||
import {
|
||||
isTransientError,
|
||||
isTransientAuthCredentialError,
|
||||
@@ -550,4 +554,47 @@ describe("Transient Error Detector", () => {
|
||||
expect(isSilentTransientError("The operation was aborted by user")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:AcpRuntime 2026-07-15-19:15 (FN-8004):
|
||||
ACP-backed runtimes (Grok, OMP, generic ACP) surface provider-side turn failures as JSON-RPC
|
||||
errors. Treating them as permanent parked a task `failed` over a ~20s blip, and since
|
||||
`status:"failed"` is what suppresses recovery, the work stranded until a human noticed.
|
||||
|
||||
The anchoring is the load-bearing part: the bare JSON-RPC text is "Internal error", which must
|
||||
NEVER match globally or it would disguise real application defects as retryable blips.
|
||||
*/
|
||||
describe("ACP provider turn failures (FN-8004)", () => {
|
||||
it("treats ACP turn failures as transient across every runtime prefix", () => {
|
||||
expect(isTransientError("Grok ACP turn failed: Internal error")).toBe(true);
|
||||
expect(isTransientError("OMP ACP turn failed: Internal error")).toBe(true);
|
||||
expect(isTransientError("Grok ACP turn failed: Internal error (acp rpc code -32603, retryable)")).toBe(true);
|
||||
});
|
||||
|
||||
it("treats retryable ACP rpc codes as transient", () => {
|
||||
for (const code of [-32603, -32000, -32001, -32002, -32003]) {
|
||||
expect(isTransientError(`Server error (acp rpc code ${code}, retryable)`)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("treats ACP startup and dead-connection diagnostics as transient", () => {
|
||||
expect(isTransientError("Grok ACP failed to start: spawn grok ENOENT")).toBe(true);
|
||||
expect(isTransientError("Grok ACP session has no live connection. The `grok agent stdio` process failed to start."))
|
||||
.toBe(true);
|
||||
});
|
||||
|
||||
it("does NOT match the bare JSON-RPC message or caller-fault codes", () => {
|
||||
expect(isTransientError("Internal error")).toBe(false);
|
||||
expect(isTransientError("Application threw Internal error while saving")).toBe(false);
|
||||
for (const code of [-32600, -32601, -32602]) {
|
||||
expect(isTransientError(`Bad call (acp rpc code ${code})`)).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("re-exports the identical predicate from the leaf module", () => {
|
||||
// The detector must stay a pure re-export — a divergent copy would let the merge
|
||||
// classifier (which imports the leaf) and the executor drift apart again.
|
||||
expect(isTransientErrorViaLeaf).toBe(isTransientError);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -39,4 +39,85 @@ describe("classifyTransientMergeError", () => {
|
||||
"Integration branch main advanced concurrently (expected aaa1111aaa1111aaa1111aaa1111aaa1111aaaa, observed bbb2222bbb2222bbb2222bbb2222bbb2222bbbb) while applying ccc3333ccc3333ccc3333ccc3333ccc3333cccc",
|
||||
)).toBeNull();
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:MergeReliability 2026-07-15-19:00 (FN-8004):
|
||||
The AI merge drives a real LLM turn, but no provider-side fault was modeled as transient, so a
|
||||
~20s Grok `-32603` blip parked a task `failed` with 8 files of finished, reviewed work stranded
|
||||
in in-review. Because `status:"failed"` is exactly what suppresses both recovery paths, the
|
||||
misclassification was self-sealing.
|
||||
|
||||
Per "Fix the Invariant, Not the Repro": assert the invariant across EVERY surface that can emit
|
||||
an ACP provider fault — not just the one reported Grok string.
|
||||
*/
|
||||
describe("ai-provider-turn-failure (FN-8004)", () => {
|
||||
it("classifies the exact error string that terminally failed FN-8004", () => {
|
||||
// Verbatim from .fusion/tasks/FN-8004/task.json `error` — the pre-fix adapter output.
|
||||
expect(classifyTransientMergeError("Grok ACP turn failed: Internal error"))
|
||||
.toBe("ai-provider-turn-failure");
|
||||
});
|
||||
|
||||
it("classifies every ACP-backed runtime's turn-failure prefix", () => {
|
||||
// Surface enumeration: all ACP adapters funnel through acp-runtime's promptAcpSession.
|
||||
for (const runtime of ["Grok", "OMP"]) {
|
||||
expect(classifyTransientMergeError(`${runtime} ACP turn failed: Internal error`))
|
||||
.toBe("ai-provider-turn-failure");
|
||||
}
|
||||
});
|
||||
|
||||
it("classifies post-fix diagnostics carrying the JSON-RPC code envelope", () => {
|
||||
expect(classifyTransientMergeError("Grok ACP turn failed: Internal error (acp rpc code -32603, retryable)"))
|
||||
.toBe("ai-provider-turn-failure");
|
||||
// Reserved server-error range -32000..-32003 is retryable too.
|
||||
for (const code of [-32000, -32001, -32002, -32003]) {
|
||||
expect(classifyTransientMergeError(`Server error (acp rpc code ${code}, retryable)`))
|
||||
.toBe("ai-provider-turn-failure");
|
||||
}
|
||||
});
|
||||
|
||||
it("does NOT swallow the bare JSON-RPC message without an ACP envelope", () => {
|
||||
// "Internal error" is far too generic to treat as transient globally — matching it
|
||||
// unanchored would mask real application defects as retryable infrastructure blips.
|
||||
expect(classifyTransientMergeError("Internal error")).toBeNull();
|
||||
expect(classifyTransientMergeError("Application threw Internal error while saving")).toBeNull();
|
||||
expect(classifyTransientMergeError("AssertionError: expected Internal error to be handled")).toBeNull();
|
||||
});
|
||||
|
||||
it("does not classify non-retryable JSON-RPC codes (caller bugs, not provider faults)", () => {
|
||||
// -32600 invalid request / -32601 method not found / -32602 invalid params are OUR bugs;
|
||||
// retrying them just repeats the failing call.
|
||||
for (const code of [-32600, -32601, -32602]) {
|
||||
expect(classifyTransientMergeError(`Bad call (acp rpc code ${code})`)).toBeNull();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:MergeReliability 2026-07-15-19:00 (FN-8004):
|
||||
The inline retry gate (`project-engine.ts#maybeRetryTransientMerge`) accepted
|
||||
`isTransientError(msg) || classify(msg)`, but the self-healing sweep
|
||||
(`recoverTransientMergeFailures`) consulted ONLY this classifier. Any network-class error
|
||||
therefore got inline retries and then became invisible to the sweep once parked `failed`.
|
||||
Delegating to `isTransientError` makes the two gates agree by construction.
|
||||
*/
|
||||
describe("network-transport-failure delegation (FN-8004 asymmetry)", () => {
|
||||
it("classifies network transport errors the sweep previously could not see", () => {
|
||||
for (const msg of ["socket hang up", "read ECONNRESET", "connect ECONNREFUSED 127.0.0.1:443", "upstream connect error"]) {
|
||||
expect(classifyTransientMergeError(msg)).toBe("network-transport-failure");
|
||||
}
|
||||
});
|
||||
|
||||
it("still returns the specific git/merge class when both could match", () => {
|
||||
// Ordering invariant: the precise class must win the audit label.
|
||||
expect(classifyTransientMergeError("Merge handoff refused (lease-handoff-failed): target-not-queued"))
|
||||
.toBe("lease-handoff-target-not-queued");
|
||||
expect(classifyTransientMergeError("spawn git ENOENT")).toBe("process-spawn-failure");
|
||||
});
|
||||
|
||||
it("leaves genuine task defects permanent", () => {
|
||||
expect(classifyTransientMergeError("Test suite failed: 3 assertions failed")).toBeNull();
|
||||
expect(classifyTransientMergeError("FileScopeViolationError: commit touches files outside scope")).toBeNull();
|
||||
expect(classifyTransientMergeError("CONFLICT (content): Merge conflict in src/app.ts")).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -541,9 +541,19 @@ export class ProjectEngine {
|
||||
}
|
||||
|
||||
/** FN-5697/FN-5674: cap transient provider/network abort retries in auto-merge.
|
||||
* Examples: "This operation was aborted", "socket hang up", `server_error`.
|
||||
* After this cap, the task is parked failed for human visibility. */
|
||||
private static readonly MAX_AUTO_MERGE_TRANSIENT_RETRIES = 3;
|
||||
* Examples: "This operation was aborted", "socket hang up", `server_error`,
|
||||
* and (FN-8004) ACP provider turn failures such as `acp rpc code -32603`.
|
||||
* After this cap, the task is parked failed for human visibility.
|
||||
*
|
||||
* FNXC:MergeReliability 2026-07-15-18:50 (FN-8004):
|
||||
* Raised 3 → 5. Applies only to errors already classified transient, and each retry
|
||||
* is spaced by exponential backoff (5s/10s/20s/40s/80s — ~2.5 min total), so the
|
||||
* widened budget rides out provider incidents lasting minutes rather than seconds
|
||||
* without meaningfully delaying a genuinely broken merge's park.
|
||||
*
|
||||
* Readable (not private) so tests derive the cap from this single source of truth rather
|
||||
* than hardcoding it — the FN-8004 bump broke two suites that had baked in the old `3`. */
|
||||
static readonly MAX_AUTO_MERGE_TRANSIENT_RETRIES = 5;
|
||||
private static readonly MERGE_REQUEST_RETRY_EXHAUSTED_AGE_MS = 30 * 60 * 1000;
|
||||
/** Cap on outer in-review→in-progress bounces caused by deterministic
|
||||
* verification failures during auto-merge. After this many failed merges
|
||||
|
||||
@@ -517,8 +517,17 @@ export const MAX_AUTO_MERGE_RETRIES = 3;
|
||||
* `mergeRetries` and re-enqueueing the same task, the task is considered
|
||||
* genuinely stuck and stays parked as `failed` for manual review. Tracked via
|
||||
* `task.mergeDetails.transientRecoveryCount`.
|
||||
*
|
||||
* FNXC:MergeReliability 2026-07-15-18:50 (FN-8004):
|
||||
* Raised 2 → 5. This budget only ever applies to errors already PROVEN transient by
|
||||
* `classifyTransientMergeError` — provider blips, network drops, lease races. For that
|
||||
* population the cost asymmetry is lopsided: an extra retry costs one merge attempt,
|
||||
* while giving up strands completed, reviewed work in `in-review` until a human notices.
|
||||
* Operators reported the old budget surrendering while the underlying fault was still
|
||||
* clearing. Recovery remains strictly bounded and audited — this widens the window,
|
||||
* it does not remove the ceiling.
|
||||
*/
|
||||
export const MAX_TRANSIENT_MERGE_RECOVERIES = 2;
|
||||
export const MAX_TRANSIENT_MERGE_RECOVERIES = 5;
|
||||
|
||||
// FN-5627: classifier extracted to `transient-merge-error-classifier.ts`
|
||||
// to avoid pulling `createLogger` into modules that mock `../logger.js`
|
||||
|
||||
@@ -16,94 +16,16 @@
|
||||
*/
|
||||
|
||||
import { isUsageLimitError } from "./usage-limit-detector.js";
|
||||
/*
|
||||
FNXC:Reliability-ErrorClassification 2026-07-15-18:40:
|
||||
The pure predicates (TRANSIENT_ERROR_PATTERNS / isTransientError / isTransientAuthCredentialError)
|
||||
now live in the import-free leaf `transient-error-patterns.ts` so the merge classifier can share
|
||||
one definition of "transient" without inheriting this module's logger chain (FN-8004).
|
||||
Re-exported here so every existing importer of this module keeps working unchanged.
|
||||
*/
|
||||
import { isTransientAuthCredentialError, isTransientError } from "./transient-error-patterns.js";
|
||||
export { TRANSIENT_ERROR_PATTERNS, isTransientAuthCredentialError, isTransientError } from "./transient-error-patterns.js";
|
||||
|
||||
/**
|
||||
* Patterns that indicate transient network/infrastructure errors.
|
||||
* These are checked case-insensitively against error messages.
|
||||
*
|
||||
* These patterns cover:
|
||||
* - Proxy/gateway connection errors (upstream connect, disconnect/reset)
|
||||
* - Connection refusal/reset (ECONNREFUSED, connection reset)
|
||||
* - Timeouts (ETIMEDOUT, timeout in connection context)
|
||||
* - Socket errors (socket hang up)
|
||||
* - Transport layer failures
|
||||
* - AI provider abort errors (request was aborted — temporary streaming/API cancellations)
|
||||
* - OpenAI/Codex infrastructure errors surfaced as structured `server_error` payloads
|
||||
*/
|
||||
export const TRANSIENT_ERROR_PATTERNS: RegExp[] = [
|
||||
// Proxy/gateway errors - indicate temporary routing issues
|
||||
/upstream connect error/i,
|
||||
/disconnect\/reset before headers/i,
|
||||
/retried and the latest reset reason/i,
|
||||
/remote connection failure/i,
|
||||
/transport failure reason/i,
|
||||
/delayed connect error/i,
|
||||
|
||||
// Connection establishment failures - usually temporary
|
||||
/Connection refused/i,
|
||||
/connection reset/i,
|
||||
/ECONNRESET/i,
|
||||
/ECONNREFUSED/i,
|
||||
/ETIMEDOUT/i,
|
||||
/socket hang up/i,
|
||||
|
||||
// Timeout patterns (only when related to connections, not general timeouts)
|
||||
/timeout.*connection/i,
|
||||
/connection.*timeout/i,
|
||||
|
||||
// AI provider abort errors — temporary request cancellations (e.g., Anthropic streaming aborts)
|
||||
// These occur when the provider's infrastructure drops an in-flight request.
|
||||
/request was aborted/i,
|
||||
// DOMException-style AbortError ("This operation was aborted"), emitted by fetch/
|
||||
// AbortController when a provider drops an in-flight operation. Excludes user-
|
||||
// initiated cancellations like "operation was aborted by user" — those are not transient.
|
||||
/operation was aborted(?!\s+by\b)/i,
|
||||
|
||||
// OpenAI/Codex structured infrastructure failures. These arrive as JSON-ish payloads
|
||||
// like {"type":"error","error":{"type":"server_error","code":"server_error",...}}
|
||||
// and are temporary service-side failures rather than task-specific defects.
|
||||
/"type":"server_error"/i,
|
||||
/"code":"server_error"/i,
|
||||
/An error occurred while processing your request\./i,
|
||||
|
||||
// pi-ai openai-codex-responses WebSocket transport errors. The provider holds
|
||||
// a long-lived WebSocket to the Codex backend; transient drops surface as
|
||||
// bare "WebSocket error" / "WebSocket closed <code> <reason>" / a half-open
|
||||
// stream that ended before `response.completed`. All three are network-layer
|
||||
// hiccups, not task defects — retry them.
|
||||
/WebSocket error\b/i,
|
||||
/WebSocket closed\b/i,
|
||||
/WebSocket stream closed before response\.completed/i,
|
||||
];
|
||||
|
||||
/**
|
||||
* Check if an error message indicates a transient network/infrastructure error.
|
||||
*
|
||||
* Transient errors are temporary conditions that typically resolve after a delay:
|
||||
* - Network blips and temporary routing issues
|
||||
* - Proxy/gateway hiccups (upstream connect errors)
|
||||
* - Connection resets during establishment
|
||||
* - Temporary service unavailability (connection refused)
|
||||
* - Socket timeouts during connection
|
||||
*
|
||||
* Returns `true` for transient errors — these should trigger a retry by moving
|
||||
* the task back to "todo" rather than marking as "failed".
|
||||
*
|
||||
* Returns `false` for permanent failures (code errors, test failures) or
|
||||
* usage limit errors (rate limits that need global pause).
|
||||
*
|
||||
* @param errorMessage - The error message to classify
|
||||
* @returns true if the error appears transient and retryable
|
||||
*/
|
||||
export function isTransientError(errorMessage: string): boolean {
|
||||
if (!errorMessage || typeof errorMessage !== "string") {
|
||||
return false;
|
||||
}
|
||||
if (isTransientAuthCredentialError(errorMessage)) {
|
||||
return true;
|
||||
}
|
||||
return TRANSIENT_ERROR_PATTERNS.some((pattern) => pattern.test(errorMessage));
|
||||
}
|
||||
|
||||
/*
|
||||
* FNXC:PlanReviewReplan 2026-07-15-12:00:
|
||||
@@ -141,34 +63,6 @@ export function isNonPlanDefectPlanReviewFailure(input: {
|
||||
);
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:Reliability-ErrorClassification 2026-07-12-20:10:
|
||||
A long-running agent session holds its OAuth access token in memory. Claude Max access tokens rotate mid-run (~8 h lifetime); the in-flight call fails with a 401 {"type":"authentication_error","message":"Invalid authentication credentials"} even though the credentials file has already been refreshed, and the very next call succeeds. These must classify as TRANSIENT (retryable) and NOT operator-actionable, so in-run retry (withRateLimitRetry) and durable-agent heartbeat error recovery (FN-7835/FN-7844/FN-7859) auto-recover instead of parking agents paused with pauseReason "error-unrecoverable". Previously the message matched the operator-actionable /credential/ and /unauthorized/ patterns and defaulted to "permanent", so a routine token rotation parked every durable agent for manual operator repair.
|
||||
Genuinely operator-actionable auth failures are excluded first: OAuth scope/permission-grant errors (token valid but lacks grants) and explicit API-key problems (invalid/missing x-api-key) — retrying those only repeats the failing call.
|
||||
*/
|
||||
const TRANSIENT_AUTH_CREDENTIAL_ROTATION_PATTERN =
|
||||
/"type":\s*"authentication_error"|invalid authentication credentials|token[_\s]?expired/i;
|
||||
/*
|
||||
FNXC:Reliability-ErrorClassification 2026-07-12-21:05:
|
||||
PR #2027 review: the `"type":"authentication_error"` envelope is intentionally broad (providers put rotation failures behind it with varying messages), so the exclusion list must carry the operator-actionable load. Beyond scope grants and invalid/missing API keys, exclude account/credential states no retry can fix: revoked/suspended/disabled/deactivated keys or accounts and inactive subscriptions. A message matching any of these stays permanent/operator-actionable even inside an authentication_error envelope; retries are pointless and would un-park agents a human must repair. Unmatched novel auth messages still classify transient, but the bounded heartbeat error-recovery budget re-parks them as `error-retry-exhausted` after a few attempts, so the failure mode is a handful of visible retries, not an unpark loop.
|
||||
*/
|
||||
const OPERATOR_ACTIONABLE_AUTH_EXCLUSION_PATTERN =
|
||||
/oauth token does not meet scope|insufficient[_\s-]?scope|invalid[_\s-]?scope|invalid (?:api[_\s-]?key|x-api-key)|missing\s+(?:\S+\s+)?(?:api[_\s-]?)?key|revoked|suspend(?:ed)?|disabled|deactivated|subscription|account (?:is )?(?:locked|closed|inactive)|access denied/i;
|
||||
|
||||
/**
|
||||
* Detect a transient authentication failure caused by credential rotation
|
||||
* (e.g. a Claude Max OAuth access token expiring mid-run). Scope-grant and
|
||||
* API-key misconfiguration errors are excluded — those need operator action.
|
||||
*/
|
||||
export function isTransientAuthCredentialError(errorMessage: string): boolean {
|
||||
if (!errorMessage || typeof errorMessage !== "string") {
|
||||
return false;
|
||||
}
|
||||
if (OPERATOR_ACTIONABLE_AUTH_EXCLUSION_PATTERN.test(errorMessage)) {
|
||||
return false;
|
||||
}
|
||||
return TRANSIENT_AUTH_CREDENTIAL_ROTATION_PATTERN.test(errorMessage);
|
||||
}
|
||||
|
||||
/**
|
||||
* Patterns for transient errors that should be silently retried without
|
||||
|
||||
153
packages/engine/src/transient-error-patterns.ts
Normal file
153
packages/engine/src/transient-error-patterns.ts
Normal file
@@ -0,0 +1,153 @@
|
||||
/**
|
||||
* Pure transient-error predicates — NO module imports.
|
||||
*
|
||||
* FNXC:Reliability-ErrorClassification 2026-07-15-18:40:
|
||||
* Extracted from `transient-error-detector.ts` (FN-8004) so that
|
||||
* `transient-merge-error-classifier.ts` can share ONE definition of "transient"
|
||||
* without inheriting the detector's `usage-limit-detector.js → logger.js` import
|
||||
* chain. FN-5627 originally split the merge classifier out precisely to keep that
|
||||
* chain away from consumers whose tests `vi.mock("../logger.js")` with a partial
|
||||
* surface (notification-service.test.ts) — importing the detector directly would
|
||||
* have silently reintroduced it.
|
||||
*
|
||||
* INVARIANT: this module must stay import-free. Anything needing `isUsageLimitError`
|
||||
* or a logger belongs in `transient-error-detector.ts`, not here.
|
||||
*
|
||||
* `transient-error-detector.ts` re-exports every symbol below, so existing importers
|
||||
* are unaffected and may continue importing from either module.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Patterns that indicate transient network/infrastructure errors.
|
||||
* These are checked case-insensitively against error messages.
|
||||
*
|
||||
* These patterns cover:
|
||||
* - Proxy/gateway connection errors (upstream connect, disconnect/reset)
|
||||
* - Connection refusal/reset (ECONNREFUSED, connection reset)
|
||||
* - Timeouts (ETIMEDOUT, timeout in connection context)
|
||||
* - Socket errors (socket hang up)
|
||||
* - Transport layer failures
|
||||
* - AI provider abort errors (request was aborted — temporary streaming/API cancellations)
|
||||
* - OpenAI/Codex infrastructure errors surfaced as structured `server_error` payloads
|
||||
*/
|
||||
export const TRANSIENT_ERROR_PATTERNS: RegExp[] = [
|
||||
// Proxy/gateway errors - indicate temporary routing issues
|
||||
/upstream connect error/i,
|
||||
/disconnect\/reset before headers/i,
|
||||
/retried and the latest reset reason/i,
|
||||
/remote connection failure/i,
|
||||
/transport failure reason/i,
|
||||
/delayed connect error/i,
|
||||
|
||||
// Connection establishment failures - usually temporary
|
||||
/Connection refused/i,
|
||||
/connection reset/i,
|
||||
/ECONNRESET/i,
|
||||
/ECONNREFUSED/i,
|
||||
/ETIMEDOUT/i,
|
||||
/socket hang up/i,
|
||||
|
||||
// Timeout patterns (only when related to connections, not general timeouts)
|
||||
/timeout.*connection/i,
|
||||
/connection.*timeout/i,
|
||||
|
||||
// AI provider abort errors — temporary request cancellations (e.g., Anthropic streaming aborts)
|
||||
// These occur when the provider's infrastructure drops an in-flight request.
|
||||
/request was aborted/i,
|
||||
// DOMException-style AbortError ("This operation was aborted"), emitted by fetch/
|
||||
// AbortController when a provider drops an in-flight operation. Excludes user-
|
||||
// initiated cancellations like "operation was aborted by user" — those are not transient.
|
||||
/operation was aborted(?!\s+by\b)/i,
|
||||
|
||||
// OpenAI/Codex structured infrastructure failures. These arrive as JSON-ish payloads
|
||||
// like {"type":"error","error":{"type":"server_error","code":"server_error",...}}
|
||||
// and are temporary service-side failures rather than task-specific defects.
|
||||
/"type":"server_error"/i,
|
||||
/"code":"server_error"/i,
|
||||
/An error occurred while processing your request\./i,
|
||||
|
||||
// pi-ai openai-codex-responses WebSocket transport errors. The provider holds
|
||||
// a long-lived WebSocket to the Codex backend; transient drops surface as
|
||||
// bare "WebSocket error" / "WebSocket closed <code> <reason>" / a half-open
|
||||
// stream that ended before `response.completed`. All three are network-layer
|
||||
// hiccups, not task defects — retry them.
|
||||
/WebSocket error\b/i,
|
||||
/WebSocket closed\b/i,
|
||||
/WebSocket stream closed before response\.completed/i,
|
||||
|
||||
/*
|
||||
FNXC:AcpRuntime 2026-07-15-18:25:
|
||||
ACP-backed runtimes (Grok, OMP, generic ACP) surface provider-side turn failures as JSON-RPC
|
||||
errors. `provider.ts#describeAcpTurnError` renders these as `... (acp rpc code -32603, retryable)`;
|
||||
the adapters wrap that as `<Runtime> ACP turn failed: ...`. Both signatures are matched here.
|
||||
|
||||
Anchoring is deliberate: the bare JSON-RPC text is "Internal error", far too generic to match
|
||||
globally (it would swallow unrelated application failures and mask real defects). We only treat
|
||||
it as transient when it carries the ACP rpc-code envelope or the adapter's turn-failure prefix.
|
||||
|
||||
FN-8004: a Grok `-32603` blip during AI merge was classified permanent, parked the task `failed`,
|
||||
and — because `status:"failed"` is what suppresses recovery — stranded 8 files of finished work.
|
||||
*/
|
||||
/\bacp rpc code -32(?:603|00[0-3])\b/i,
|
||||
/\bACP turn failed\b/i,
|
||||
/\bACP failed to start\b/i,
|
||||
/\bACP session has no live connection\b/i,
|
||||
];
|
||||
|
||||
/**
|
||||
* Check if an error message indicates a transient network/infrastructure error.
|
||||
*
|
||||
* Transient errors are temporary conditions that typically resolve after a delay:
|
||||
* - Network blips and temporary routing issues
|
||||
* - Proxy/gateway hiccups (upstream connect errors)
|
||||
* - Connection resets during establishment
|
||||
* - Temporary service unavailability (connection refused)
|
||||
* - Socket timeouts during connection
|
||||
*
|
||||
* Returns `true` for transient errors — these should trigger a retry by moving
|
||||
* the task back to "todo" rather than marking as "failed".
|
||||
*
|
||||
* Returns `false` for permanent failures (code errors, test failures) or
|
||||
* usage limit errors (rate limits that need global pause).
|
||||
*
|
||||
* @param errorMessage - The error message to classify
|
||||
* @returns true if the error appears transient and retryable
|
||||
*/
|
||||
export function isTransientError(errorMessage: string): boolean {
|
||||
if (!errorMessage || typeof errorMessage !== "string") {
|
||||
return false;
|
||||
}
|
||||
if (isTransientAuthCredentialError(errorMessage)) {
|
||||
return true;
|
||||
}
|
||||
return TRANSIENT_ERROR_PATTERNS.some((pattern) => pattern.test(errorMessage));
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:Reliability-ErrorClassification 2026-07-12-20:10:
|
||||
A long-running agent session holds its OAuth access token in memory. Claude Max access tokens rotate mid-run (~8 h lifetime); the in-flight call fails with a 401 {"type":"authentication_error","message":"Invalid authentication credentials"} even though the credentials file has already been refreshed, and the very next call succeeds. These must classify as TRANSIENT (retryable) and NOT operator-actionable, so in-run retry (withRateLimitRetry) and durable-agent heartbeat error recovery (FN-7835/FN-7844/FN-7859) auto-recover instead of parking agents paused with pauseReason "error-unrecoverable". Previously the message matched the operator-actionable /credential/ and /unauthorized/ patterns and defaulted to "permanent", so a routine token rotation parked every durable agent for manual operator repair.
|
||||
Genuinely operator-actionable auth failures are excluded first: OAuth scope/permission-grant errors (token valid but lacks grants) and explicit API-key problems (invalid/missing x-api-key) — retrying those only repeats the failing call.
|
||||
*/
|
||||
const TRANSIENT_AUTH_CREDENTIAL_ROTATION_PATTERN =
|
||||
/"type":\s*"authentication_error"|invalid authentication credentials|token[_\s]?expired/i;
|
||||
/*
|
||||
FNXC:Reliability-ErrorClassification 2026-07-12-21:05:
|
||||
PR #2027 review: the `"type":"authentication_error"` envelope is intentionally broad (providers put rotation failures behind it with varying messages), so the exclusion list must carry the operator-actionable load. Beyond scope grants and invalid/missing API keys, exclude account/credential states no retry can fix: revoked/suspended/disabled/deactivated keys or accounts and inactive subscriptions. A message matching any of these stays permanent/operator-actionable even inside an authentication_error envelope; retries are pointless and would un-park agents a human must repair. Unmatched novel auth messages still classify transient, but the bounded heartbeat error-recovery budget re-parks them as `error-retry-exhausted` after a few attempts, so the failure mode is a handful of visible retries, not an unpark loop.
|
||||
*/
|
||||
const OPERATOR_ACTIONABLE_AUTH_EXCLUSION_PATTERN =
|
||||
/oauth token does not meet scope|insufficient[_\s-]?scope|invalid[_\s-]?scope|invalid (?:api[_\s-]?key|x-api-key)|missing\s+(?:\S+\s+)?(?:api[_\s-]?)?key|revoked|suspend(?:ed)?|disabled|deactivated|subscription|account (?:is )?(?:locked|closed|inactive)|access denied/i;
|
||||
|
||||
/**
|
||||
* Detect a transient authentication failure caused by credential rotation
|
||||
* (e.g. a Claude Max OAuth access token expiring mid-run). Scope-grant and
|
||||
* API-key misconfiguration errors are excluded — those need operator action.
|
||||
*/
|
||||
export function isTransientAuthCredentialError(errorMessage: string): boolean {
|
||||
if (!errorMessage || typeof errorMessage !== "string") {
|
||||
return false;
|
||||
}
|
||||
if (OPERATOR_ACTIONABLE_AUTH_EXCLUSION_PATTERN.test(errorMessage)) {
|
||||
return false;
|
||||
}
|
||||
return TRANSIENT_AUTH_CREDENTIAL_ROTATION_PATTERN.test(errorMessage);
|
||||
}
|
||||
@@ -42,12 +42,41 @@
|
||||
* stale temp merge checkout), not that the task branch's code failed. A
|
||||
* fresh merge attempt gets a fresh/revalidated worktree, so the self-healing
|
||||
* sweep can recover these within its bounded retry budget.
|
||||
*
|
||||
* - `ai-provider-turn-failure`: the AI merge's LLM turn failed provider-side
|
||||
* (FN-8004). The merger drives a real model to resolve/compose the squash;
|
||||
* when that provider returns an internal/server error, the *merge* failed but
|
||||
* the task branch is untouched and a fresh attempt typically succeeds. Before
|
||||
* FN-8004 no provider fault was modeled here at all, so every one of them was
|
||||
* treated as a permanent defect.
|
||||
*
|
||||
* - `network-transport-failure`: delegated to `isTransientError` (see below).
|
||||
*/
|
||||
// Imports the import-free leaf, NOT `transient-error-detector.js` — that module pulls
|
||||
// `usage-limit-detector.js → logger.js`, the exact chain FN-5627 split this file out to avoid.
|
||||
import { isTransientError } from "./transient-error-patterns.js";
|
||||
|
||||
/*
|
||||
FNXC:MergeReliability 2026-07-15-18:30:
|
||||
This classifier used to recognize only git/lease/spawn faults, while the inline retry gate in
|
||||
`project-engine.ts#maybeRetryTransientMerge` accepted `isTransientError(msg) || classify(msg)`.
|
||||
The self-healing sweep (`recoverTransientMergeFailures`) consulted ONLY this classifier — so any
|
||||
network-class error (ECONNRESET, socket hang up, WebSocket drop) got inline retries but became
|
||||
invisible to the sweep once parked `failed`, stranding it forever.
|
||||
|
||||
Delegating to `isTransientError` here makes the two gates agree by construction. Both consumers
|
||||
now see one definition of "transient", which is what the FN-5627 header above already claimed.
|
||||
*/
|
||||
export function classifyTransientMergeError(error: string | null | undefined): string | null {
|
||||
if (!error) return null;
|
||||
if (/lease-handoff-failed[^a-z]+target-not-queued/i.test(error)) {
|
||||
return "lease-handoff-target-not-queued";
|
||||
}
|
||||
// FNXC:MergeReliability 2026-07-15-18:30 (FN-8004): AI-merge provider faults precede the
|
||||
// generic network check so the more specific class wins in the audit/log trail.
|
||||
if (/\bACP turn failed\b/i.test(error) || /\bacp rpc code -32(?:603|00[0-3])\b/i.test(error)) {
|
||||
return "ai-provider-turn-failure";
|
||||
}
|
||||
if (/\bspawn(?:\s+\S+)?\s+ENO(?:TDIR|ENT)\b/i.test(error)) {
|
||||
return "process-spawn-failure";
|
||||
}
|
||||
@@ -58,5 +87,10 @@ export function classifyTransientMergeError(error: string | null | undefined): s
|
||||
if (sameSha && sameSha[1].toLowerCase() === sameSha[2].toLowerCase()) {
|
||||
return "spurious-concurrent-advance-same-sha";
|
||||
}
|
||||
// FNXC:MergeReliability 2026-07-15-18:30 (FN-8004): last — the specific git/merge classes above
|
||||
// must win the label. This aligns the sweep with the inline retry gate (see header).
|
||||
if (isTransientError(error)) {
|
||||
return "network-transport-failure";
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
/*
|
||||
FNXC:AcpRuntime 2026-07-15-19:10 (FN-8004):
|
||||
An ACP turn failing server-side arrives as a JSON-RPC error whose `message` is the bare
|
||||
protocol-standard text — `-32603` renders as literally "Internal error". Rethrowing it unchanged
|
||||
discarded `code`/`data`, the only evidence the fault was provider-side and retryable. The engine's
|
||||
transient classifier then saw an unclassifiable string and parked the task permanently, stranding
|
||||
completed work (FN-8004: a ~20s Grok blip terminally failed an auto-merge).
|
||||
|
||||
These tests pin the diagnostic SHAPE, because `transient-error-patterns.ts` and
|
||||
`transient-merge-error-classifier.ts` match on it. Changing the format here without updating those
|
||||
regexes silently reintroduces the FN-8004 stranding — the string is a cross-package contract.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describeAcpTurnError, inspectAcpTurnError, promptAcpSession } from "../provider.js";
|
||||
|
||||
/** Flat `{ code, message, data }` — one of two shapes the SDK throws depending on build. */
|
||||
function flatRpcError(code: number, message: string, data?: unknown): Error {
|
||||
return Object.assign(new Error(message), { code, ...(data === undefined ? {} : { data }) });
|
||||
}
|
||||
|
||||
/** Nested `{ error: { code, message } }` — the other observed SDK shape. */
|
||||
function nestedRpcError(code: number, message: string, data?: unknown): Error {
|
||||
return Object.assign(new Error("request failed"), {
|
||||
error: { code, message, ...(data === undefined ? {} : { data }) },
|
||||
});
|
||||
}
|
||||
|
||||
describe("inspectAcpTurnError", () => {
|
||||
it("extracts code and retryability from a flat JSON-RPC error", () => {
|
||||
const detail = inspectAcpTurnError(flatRpcError(-32603, "Internal error"));
|
||||
expect(detail).toMatchObject({ message: "Internal error", code: -32603, retryable: true });
|
||||
});
|
||||
|
||||
it("extracts code from the nested { error: { ... } } shape", () => {
|
||||
const detail = inspectAcpTurnError(nestedRpcError(-32603, "Internal error"));
|
||||
expect(detail).toMatchObject({ message: "Internal error", code: -32603, retryable: true });
|
||||
});
|
||||
|
||||
it("marks caller-fault codes non-retryable", () => {
|
||||
// -32600/-32601/-32602 mean WE sent a bad request; retrying repeats the failure.
|
||||
for (const code of [-32600, -32601, -32602]) {
|
||||
expect(inspectAcpTurnError(flatRpcError(code, "Invalid request")).retryable).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("marks the provider server-error range retryable", () => {
|
||||
for (const code of [-32603, -32000, -32001, -32002, -32003]) {
|
||||
expect(inspectAcpTurnError(flatRpcError(code, "Server error")).retryable).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("degrades gracefully on non-RPC errors and non-Error throws", () => {
|
||||
expect(inspectAcpTurnError(new Error("boom"))).toMatchObject({ message: "boom", code: undefined, retryable: false });
|
||||
expect(inspectAcpTurnError("plain string")).toMatchObject({ message: "plain string", retryable: false });
|
||||
expect(inspectAcpTurnError(null)).toMatchObject({ message: "unknown error", retryable: false });
|
||||
});
|
||||
});
|
||||
|
||||
describe("describeAcpTurnError", () => {
|
||||
it("renders the rpc code so downstream classifiers can anchor on it", () => {
|
||||
expect(describeAcpTurnError(flatRpcError(-32603, "Internal error")))
|
||||
.toBe("Internal error (acp rpc code -32603, retryable)");
|
||||
});
|
||||
|
||||
it("omits the retryable marker for caller-fault codes", () => {
|
||||
expect(describeAcpTurnError(flatRpcError(-32602, "Invalid params")))
|
||||
.toBe("Invalid params (acp rpc code -32602)");
|
||||
});
|
||||
|
||||
it("appends structured data when the provider supplies a cause", () => {
|
||||
const out = describeAcpTurnError(flatRpcError(-32603, "Internal error", { reason: "upstream timeout" }));
|
||||
expect(out).toContain("acp rpc code -32603, retryable");
|
||||
expect(out).toContain('[data: {"reason":"upstream timeout"}]');
|
||||
});
|
||||
|
||||
it("leaves plain errors untouched so unrelated failures are not disguised as ACP faults", () => {
|
||||
expect(describeAcpTurnError(new Error("Test suite failed"))).toBe("Test suite failed");
|
||||
});
|
||||
|
||||
it("survives unserializable data without throwing", () => {
|
||||
const circular: Record<string, unknown> = {};
|
||||
circular.self = circular;
|
||||
expect(() => describeAcpTurnError(flatRpcError(-32603, "Internal error", circular))).not.toThrow();
|
||||
});
|
||||
|
||||
it("bounds oversized data payloads", () => {
|
||||
const out = describeAcpTurnError(flatRpcError(-32603, "Internal error", { blob: "x".repeat(5_000) }));
|
||||
expect(out.length).toBeLessThan(700);
|
||||
});
|
||||
});
|
||||
|
||||
describe("promptAcpSession error propagation", () => {
|
||||
const session = { conn: { prompt: async () => ({ stopReason: "end_turn" }) } };
|
||||
|
||||
it("returns the stopReason on success", async () => {
|
||||
await expect(promptAcpSession(session as never, "s1", [])).resolves.toBe("end_turn");
|
||||
});
|
||||
|
||||
it("rethrows provider faults with the rpc code preserved in the message", async () => {
|
||||
const failing = { conn: { prompt: async () => { throw flatRpcError(-32603, "Internal error"); } } };
|
||||
// The regression: this message previously read only "Internal error".
|
||||
await expect(promptAcpSession(failing as never, "s1", [])).rejects.toThrow(
|
||||
"Internal error (acp rpc code -32603, retryable)",
|
||||
);
|
||||
});
|
||||
|
||||
it("retains the original error as `cause` for debugging", async () => {
|
||||
const original = flatRpcError(-32603, "Internal error");
|
||||
const failing = { conn: { prompt: async () => { throw original; } } };
|
||||
await expect(promptAcpSession(failing as never, "s1", [])).rejects.toMatchObject({ cause: original });
|
||||
});
|
||||
});
|
||||
@@ -489,20 +489,112 @@ export async function newAcpSession(
|
||||
return { sessionId: boundIdentifier(res.sessionId), modes: res.modes ?? undefined };
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:AcpRuntime 2026-07-15-18:20:
|
||||
An ACP turn that fails server-side arrives as a JSON-RPC error object whose `message` is the
|
||||
bare protocol-standard text — `-32603` renders as literally "Internal error". Rethrowing the
|
||||
SDK error as-is discards `code`/`data`, the only fields identifying the fault as provider-side
|
||||
and retryable. Downstream transient classification then sees an unclassifiable string and parks
|
||||
the task permanently (FN-8004: a 20s Grok blip terminally failed an auto-merge).
|
||||
|
||||
`describeAcpTurnError` re-shapes the error into a stable, greppable diagnostic carrying the
|
||||
numeric code, so classifiers anchor on an ACP-specific signature instead of pattern-matching the
|
||||
dangerously generic phrase "Internal error" (which could appear in unrelated application output).
|
||||
Format is load-bearing — `ACP_TRANSIENT_ERROR_PATTERNS` in the engine's transient-error-detector
|
||||
matches it. Keep the two in sync.
|
||||
*/
|
||||
|
||||
/** JSON-RPC error codes that indicate a provider-side, retryable fault rather than a bad request. */
|
||||
const RETRYABLE_JSONRPC_CODES = new Set([
|
||||
-32603, // Internal error — the agent blew up server-side.
|
||||
-32000, // Server error (generic, reserved implementation-defined range).
|
||||
-32001,
|
||||
-32002,
|
||||
-32003,
|
||||
]);
|
||||
|
||||
/** Structured view of a thrown ACP/JSON-RPC error; all fields best-effort. */
|
||||
export interface AcpTurnErrorDetail {
|
||||
message: string;
|
||||
code?: number;
|
||||
data?: unknown;
|
||||
retryable: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract JSON-RPC `code`/`data` from a thrown ACP error.
|
||||
*
|
||||
* The SDK surfaces request errors in more than one shape depending on build: a flat
|
||||
* `{ code, message, data }` and a nested `{ error: { code, message, data } }`. Read both.
|
||||
*/
|
||||
export function inspectAcpTurnError(error: unknown): AcpTurnErrorDetail {
|
||||
type RpcShape = { code?: unknown; data?: unknown; message?: unknown };
|
||||
const raw = error as (RpcShape & { error?: RpcShape }) | null;
|
||||
const nested = raw && typeof raw === "object" ? raw.error : undefined;
|
||||
|
||||
/*
|
||||
Read message/code/data from ONE source. Reading `code` from the nested payload while taking
|
||||
`message` from the outer Error yields "request failed (acp rpc code -32603)" — the outer
|
||||
wrapper text, not the provider's actual reason. The flat shape wins when it carries a numeric
|
||||
code; otherwise the nested envelope is authoritative.
|
||||
*/
|
||||
const source: RpcShape | undefined =
|
||||
typeof raw?.code === "number" ? raw : typeof nested?.code === "number" ? nested : undefined;
|
||||
|
||||
const code = typeof source?.code === "number" ? source.code : undefined;
|
||||
const data = source?.data;
|
||||
const message =
|
||||
(typeof source?.message === "string" && source.message)
|
||||
|| (typeof raw?.message === "string" && raw.message)
|
||||
|| (error instanceof Error ? error.message : String(error ?? "unknown error"));
|
||||
|
||||
return { message, code, data, retryable: code !== undefined && RETRYABLE_JSONRPC_CODES.has(code) };
|
||||
}
|
||||
|
||||
/**
|
||||
* Render a thrown ACP turn error as a single-line diagnostic that preserves the JSON-RPC code
|
||||
* and any `data` payload, so transient classification has something to anchor on.
|
||||
*
|
||||
* Shape: `Internal error (acp rpc code -32603, retryable) [data: {...}]`
|
||||
*/
|
||||
export function describeAcpTurnError(error: unknown): string {
|
||||
const { message, code, data, retryable } = inspectAcpTurnError(error);
|
||||
if (code === undefined) return message;
|
||||
const suffix = retryable ? ", retryable" : "";
|
||||
let out = `${message} (acp rpc code ${code}${suffix})`;
|
||||
if (data !== undefined) {
|
||||
let rendered: string;
|
||||
try {
|
||||
rendered = typeof data === "string" ? data : JSON.stringify(data);
|
||||
} catch {
|
||||
rendered = String(data);
|
||||
}
|
||||
if (rendered && rendered !== "{}") out += ` [data: ${rendered.slice(0, 500)}]`;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Send a prompt turn via `session/prompt` and return the terminal `stopReason`.
|
||||
*
|
||||
* The SDK prompt promise resolves only AFTER every `session/update` for the turn
|
||||
* has been delivered to the client handler — so resolving here is the correct
|
||||
* "turn complete" signal (no extra draining required).
|
||||
*
|
||||
* FNXC:AcpRuntime 2026-07-15-18:20: rethrows with `describeAcpTurnError` so the JSON-RPC code
|
||||
* survives to the engine's transient classifier (FN-8004). `cause` retains the original error.
|
||||
*/
|
||||
export async function promptAcpSession(
|
||||
connection: AcpConnection,
|
||||
sessionId: string,
|
||||
blocks: ContentBlock[],
|
||||
): Promise<StopReason> {
|
||||
const res = await connection.conn.prompt({ sessionId, prompt: blocks });
|
||||
return res.stopReason;
|
||||
try {
|
||||
const res = await connection.conn.prompt({ sessionId, prompt: blocks });
|
||||
return res.stopReason;
|
||||
} catch (error) {
|
||||
throw new Error(describeAcpTurnError(error), { cause: error });
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -89,6 +89,16 @@ function describeCreateFailure(error: unknown): string {
|
||||
);
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:GrokAcp 2026-07-15-18:45:
|
||||
`promptAcpSession` (acp-runtime provider.ts) already re-shapes JSON-RPC faults into a diagnostic
|
||||
carrying the rpc code — e.g. `Internal error (acp rpc code -32603, retryable)`. Pass that through
|
||||
verbatim rather than re-flattening to `error.message`, so the engine's transient classifier can
|
||||
recognize a provider-side blip and retry instead of parking the task permanently.
|
||||
|
||||
FN-8004: the bare message reaching the merger was "Internal error", matched no transient pattern,
|
||||
and terminally failed an auto-merge whose branch work was complete and correct.
|
||||
*/
|
||||
function describePromptFailure(error: unknown): string {
|
||||
const reason = error instanceof Error ? error.message : String(error ?? "unknown error");
|
||||
return compactDiagnostic(`Grok ACP turn failed: ${reason}`);
|
||||
|
||||
Reference in New Issue
Block a user