Feat/model scheduler probe routing#2434
Conversation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement (CLA). To sign the CLA, please comment on this PR with: This is a one-time requirement. Once signed, all your future contributions will be automatically accepted. I have read the CLA Document and I hereby sign the CLA Calvin Cyan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
There was a problem hiding this comment.
6 issues found across 25 files
Confidence score: 2/5
- I’m scoring this as high risk because there are several high-confidence medium/high-severity issues (6–7/10) with concrete runtime impact rather than style-only concerns.
- The most severe behavior risk is in
src/features/model-scheduler/scheduler.ts: cooldown state can be dropped after one stable cycle, which can cause recently changed models to be reconsidered too early and destabilize selection. src/shared/opencode-config-reader.tshas two user-facing config hazards: bypassing OpenCode’s normal config resolution chain and mis-handling array-basedmodelsviaObject.keys(), which can produce incorrect model whitelisting.- Pay close attention to
src/features/model-scheduler/scheduler.ts,src/shared/opencode-config-reader.ts,src/features/model-scheduler/selector.ts,src/features/model-scheduler/candidate-models.ts, andsrc/features/model-scheduler/health-store.ts- scheduler correctness, config interpretation, contradictory availability reasons, and duplicated helper logic need validation before merge.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/shared/opencode-config-reader.ts">
<violation number="1" location="src/shared/opencode-config-reader.ts:26">
P1: Custom agent: **Opencode Compatibility**
Manually reading the global `opencode.json(c)` bypasses OpenCode's configuration resolution hierarchy. OpenCode merges configurations from multiple sources (including project-level `opencode.json` and `.well-known/opencode` defaults) and performs environment variable substitution. To ensure compatibility, use the SDK's `await client.config.get()` to retrieve the fully resolved configuration.</violation>
<violation number="2" location="src/shared/opencode-config-reader.ts:68">
P2: Configuring `models` as a JSON array silently whitelists array indices ("0", "1", etc.) instead of actual model names. When `Object.keys()` is called on an array, it returns string indices. The logging will show the correct count (array length), but model routing will fail because the IDs don't match real models.</violation>
</file>
<file name="src/features/model-scheduler/candidate-models.ts">
<violation number="1" location="src/features/model-scheduler/candidate-models.ts:8">
P1: Duplicate `normalizeKey` function introduced in two files</violation>
</file>
<file name="src/features/model-scheduler/health-store.ts">
<violation number="1" location="src/features/model-scheduler/health-store.ts:14">
P1: Duplicate `writeJsonAtomic` function across two new files</violation>
</file>
<file name="src/features/model-scheduler/selector.ts">
<violation number="1" location="src/features/model-scheduler/selector.ts:39">
P2: Contradictory `reason: "unavailable"` returned when current model is healthy</violation>
</file>
<file name="src/features/model-scheduler/scheduler.ts">
<violation number="1" location="src/features/model-scheduler/scheduler.ts:166">
P1: Active model cooldowns are prematurely discarded after a single cycle. The `getCooldownUntil` function only returns a cooldown timestamp when `previous?.changed` is true. After a change occurs and one stable cycle passes, `previous.changed` becomes false, causing `getCooldownUntil` to return `undefined`. The `cooldownUntil` field is then omitted from the health record, permanently discarding any unexpired cooldown. This renders the `agent_cooldown_minutes` configuration ineffective.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,85 @@ | |||
| import { existsSync, readFileSync } from "fs" | |||
There was a problem hiding this comment.
P1: Custom agent: Opencode Compatibility
Manually reading the global opencode.json(c) bypasses OpenCode's configuration resolution hierarchy. OpenCode merges configurations from multiple sources (including project-level opencode.json and .well-known/opencode defaults) and performs environment variable substitution. To ensure compatibility, use the SDK's await client.config.get() to retrieve the fully resolved configuration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/shared/opencode-config-reader.ts, line 26:
<comment>Manually reading the global `opencode.json(c)` bypasses OpenCode's configuration resolution hierarchy. OpenCode merges configurations from multiple sources (including project-level `opencode.json` and `.well-known/opencode` defaults) and performs environment variable substitution. To ensure compatibility, use the SDK's `await client.config.get()` to retrieve the fully resolved configuration.</comment>
<file context>
@@ -0,0 +1,85 @@
+ * const whitelist = readUserConfiguredModels()
+ * // → Map { "minimax" => Set { "MiniMax-M2.5-highspeed" } }
+ */
+export function readUserConfiguredModels(): Map<string, Set<string>> | null {
+ const configDir = getOpenCodeConfigDir({ binary: "opencode", version: null })
+ const configPaths = [
</file context>
| @@ -0,0 +1,59 @@ | |||
| import { | |||
There was a problem hiding this comment.
P1: Duplicate normalizeKey function introduced in two files
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/model-scheduler/candidate-models.ts, line 8:
<comment>Duplicate `normalizeKey` function introduced in two files</comment>
<file context>
@@ -0,0 +1,59 @@
+} from "../../shared"
+import type { RoutingEntry, RoutingTargetKind } from "./types"
+
+function normalizeKey(value: string): string {
+ return value.trim().toLowerCase().replace(/[^a-z0-9]+/g, "-").replace(/^-+|-+$/g, "")
+}
</file context>
| @@ -0,0 +1,60 @@ | |||
| import { existsSync, mkdirSync, readFileSync, renameSync, unlinkSync, writeFileSync } from "node:fs" | |||
There was a problem hiding this comment.
P1: Duplicate writeJsonAtomic function across two new files
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/model-scheduler/health-store.ts, line 14:
<comment>Duplicate `writeJsonAtomic` function across two new files</comment>
<file context>
@@ -0,0 +1,60 @@
+ }
+}
+
+function writeJsonAtomic(filePath: string, data: unknown): void {
+ ensureParentDir(filePath)
+ const tempPath = `${filePath}.tmp.${Date.now()}`
</file context>
|
|
||
| function getCooldownUntil(previous: RoutingTargetHealth | null, cooldownMinutes: number): string | undefined { | ||
| if (cooldownMinutes <= 0) return undefined | ||
| if (!previous?.changed) return undefined |
There was a problem hiding this comment.
P1: Active model cooldowns are prematurely discarded after a single cycle. The getCooldownUntil function only returns a cooldown timestamp when previous?.changed is true. After a change occurs and one stable cycle passes, previous.changed becomes false, causing getCooldownUntil to return undefined. The cooldownUntil field is then omitted from the health record, permanently discarding any unexpired cooldown. This renders the agent_cooldown_minutes configuration ineffective.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/model-scheduler/scheduler.ts, line 166:
<comment>Active model cooldowns are prematurely discarded after a single cycle. The `getCooldownUntil` function only returns a cooldown timestamp when `previous?.changed` is true. After a change occurs and one stable cycle passes, `previous.changed` becomes false, causing `getCooldownUntil` to return `undefined`. The `cooldownUntil` field is then omitted from the health record, permanently discarding any unexpired cooldown. This renders the `agent_cooldown_minutes` configuration ineffective.</comment>
<file context>
@@ -0,0 +1,483 @@
+
+function getCooldownUntil(previous: RoutingTargetHealth | null, cooldownMinutes: number): string | undefined {
+ if (cooldownMinutes <= 0) return undefined
+ if (!previous?.changed) return undefined
+ const baseTime = previous.checkedAt ? Date.parse(previous.checkedAt) : Number.NaN
+ if (Number.isNaN(baseTime)) return undefined
</file context>
|
|
||
| const healthyCurrent = resolveHealthyModel(currentModel, availableModels) | ||
| if (healthyCurrent) { | ||
| return { model: healthyCurrent, reason: "unavailable" } |
There was a problem hiding this comment.
P2: Contradictory reason: "unavailable" returned when current model is healthy
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/model-scheduler/selector.ts, line 39:
<comment>Contradictory `reason: "unavailable"` returned when current model is healthy</comment>
<file context>
@@ -0,0 +1,88 @@
+
+ const healthyCurrent = resolveHealthyModel(currentModel, availableModels)
+ if (healthyCurrent) {
+ return { model: healthyCurrent, reason: "unavailable" }
+ }
+
</file context>
| continue | ||
| } | ||
|
|
||
| const modelIds = Object.keys(providerConfig.models) |
There was a problem hiding this comment.
P2: Configuring models as a JSON array silently whitelists array indices ("0", "1", etc.) instead of actual model names. When Object.keys() is called on an array, it returns string indices. The logging will show the correct count (array length), but model routing will fail because the IDs don't match real models.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/shared/opencode-config-reader.ts, line 68:
<comment>Configuring `models` as a JSON array silently whitelists array indices ("0", "1", etc.) instead of actual model names. When `Object.keys()` is called on an array, it returns string indices. The logging will show the correct count (array length), but model routing will fail because the IDs don't match real models.</comment>
<file context>
@@ -0,0 +1,85 @@
+ continue
+ }
+
+ const modelIds = Object.keys(providerConfig.models)
+ if (modelIds.length > 0) {
+ whitelist.set(providerId, new Set(modelIds))
</file context>
Summary
Verification
Summary by cubic
Adds a probe-aware model scheduler that routes agents and categories to healthy models. It runs on session startup and at a fixed interval, and respects user-configured provider model sets.
New Features
model-health.jsonandscheduler-audit.jsonl(cache dir) and updatesmodel-routing.jsonwith scheduler metadata and refreshed fallbacks.model_schedulerblock (enabled, interval_minutes, mode, failure/recovery thresholds, agent_cooldown_minutes, protect_manual_routing, probe_enabled, probe_timeout_ms, probe_max_latency_ms). Types exported.session.createdand starts a background loop atgetModelSchedulerIntervalMs(...).opencode.json/opencode.jsoncwhen listing available models.Dependencies
oh-my-opencode-*platform packages from 3.10.0 to 3.10.1.Written for commit 9254706. Summary will update on new commits.