From 56f4800197eabd6ff314e6cd27dc939532b7b704 Mon Sep 17 00:00:00 2001 From: ShutovKS Date: Wed, 15 Jul 2026 00:06:10 +0300 Subject: [PATCH] fix: address Copilot review feedback from PRs #2 and #3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #3 (suppressed comment): errorText only walked `cause` for Error instances; plain-object wrappers fell through to JSON.stringify, which serializes a nested Error to {} and drops retryable text. Walk `cause` explicitly for records too, with the same depth cap. PR #2 (inline comment): the prune test only proved an expired cooldown made the model selectable again — pruning itself was unobservable. Expose active cooldowns as `cooling` in model_fallback_control status and assert the expired entries are gone. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 7 ++++++- README.md | 2 +- src/plugin.ts | 10 ++++++++++ test/index.test.ts | 22 ++++++++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfab119..a56122a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,9 @@ behavior unless noted. - **Configurable retry patterns.** New `retry_on_patterns` option appends user-supplied case-insensitive regex sources to the built-in patterns. Invalid sources are skipped rather than failing plugin init. (#4) +- **Active cooldowns in `status`.** `model_fallback_control status` now + reports `cooling` (model → cooldown-expiry timestamp), making cooldown + pruning observable. ### Changed @@ -46,7 +49,9 @@ behavior unless noted. - **Retryable errors are now detected through `error.cause`.** `statusCode` and `errorText` walk the cause chain (bounded to depth 5, safe against cycles), so a 429/503 buried in a wrapped fetch error triggers fallback. - (#3) + Plain-object wrappers whose `cause` is an `Error` are also handled — + `JSON.stringify` serializes nested `Error`s to `{}`, which previously + dropped retryable text. (#3) ### Fixed diff --git a/README.md b/README.md index 4b455f7..e487754 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ Actions: - `enable`: enable fallback for the current session. - `disable`: disable fallback for the current session. -- `status`: return effective state, current model, attempts, windowed attempt count, per-model `failureCounts`, recent `switches`, and configured fallbacks. +- `status`: return effective state, current model, attempts, windowed attempt count, per-model `failureCounts`, active `cooling` cooldowns (model → expiry timestamp), recent `switches`, and configured fallbacks. - `reset`: clear session state and return to the config default. Session overrides are in memory only. They disappear when the session is deleted or OpenCode restarts. diff --git a/src/plugin.ts b/src/plugin.ts index 4e7a14c..2fea318 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -266,6 +266,15 @@ function errorText(error: unknown, depth = 0): string { return text } + // Plain objects: JSON.stringify drops nested Error values (they serialize + // to {}), so walk `cause` explicitly with the same depth cap. + if (isRecord(error) && depth < MAX_CAUSE_DEPTH && error.cause !== undefined) { + const { cause, ...rest } = error + const causeText = errorText(cause, depth + 1) + const ownText = errorText(rest, MAX_CAUSE_DEPTH) + return causeText ? `${ownText} ${causeText}` : ownText + } + try { return JSON.stringify(error) } catch { @@ -436,6 +445,7 @@ function statusText(config: Config, state: SessionState | undefined): string { fallbackModels: config.fallbackModels, unavailableModels: config.unavailableModels, failureCounts: state ? Object.fromEntries(state.failureCounts) : {}, + cooling: state ? Object.fromEntries(state.failedUntil) : {}, switches: state?.switches ?? [], }) } diff --git a/test/index.test.ts b/test/index.test.ts index 832b2b4..58a1131 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -516,6 +516,16 @@ describe("model fallback plugin", () => { await emitSessionError(plugin, { providerID: "anthropic", modelID: "claude-sonnet" }) expect(prompts).toHaveLength(3) expect(prompts[2]?.body.model).toEqual({ providerID: "openai", modelID: "gpt-5.1-codex" }) + + // pruning is observable through status: the expired opus/codex cooldowns + // were removed, only the fresh sonnet cooldown remains + const status = await plugin.tool.model_fallback_control.execute({ action: "status" }, { + sessionID: "ses_1", messageID: "msg_tool", agent: "build", + directory: "/repo", worktree: "/repo", + abort: new AbortController().signal, + metadata: () => undefined, ask: async () => undefined, + }) + expect(Object.keys(JSON.parse(String(status)).cooling)).toEqual(["anthropic/claude-sonnet"]) }) test("#given first fallback is also unavailable #when retrying a failed model #then it skips to the next fallback", async () => { @@ -1028,6 +1038,18 @@ describe("helpers", () => { expect(isRetryableError(err, [])).toBe(true) }) + test("#given a plain object whose cause is an Error with retryable text #when classified #then it is retryable", () => { + // JSON.stringify(new Error(...)) yields "{}", so the text would be lost + // without explicit cause traversal for non-Error wrappers + expect(isRetryableError({ message: "request failed", cause: new Error("model is overloaded") }, [])).toBe(true) + }) + + test("#given a circular plain-object cause chain #when classified by text #then it terminates without matching", () => { + const err: Record = { message: "boom" } + err.cause = { cause: err } + expect(isRetryableError(err, [])).toBe(false) + }) + test("#given a circular cause chain #when classified #then it terminates without matching", () => { const err: Record = { message: "boom" } err.cause = err