Skip to content

fix(cron): honor agentRuntime.id when stamping cron session modelProvider#75840

Closed
vishutdhar wants to merge 1 commit intoopenclaw:mainfrom
vishutdhar:fix/cron-session-cli-provider
Closed

fix(cron): honor agentRuntime.id when stamping cron session modelProvider#75840
vishutdhar wants to merge 1 commit intoopenclaw:mainfrom
vishutdhar:fix/cron-session-cli-provider

Conversation

@vishutdhar
Copy link
Copy Markdown
Contributor

@vishutdhar vishutdhar commented May 1, 2026 •
edited
Loading

Bug

resolveCronModelSelection derives provider purely from the model-string slash-prefix (e.g. anthropic/claude-opus-4-7 -> anthropic), ignoring agents.defaults.agentRuntime.id. For agents whose runtime is a CLI backend (e.g. claude-cli), this stamps modelProvider="anthropic" on the cron session and routes cron-fired turns through the paid Anthropic API instead of the OAuth-backed Claude CLI.

The interactive path already consults isCliProvider() (src/cron/isolated-agent/run-executor.ts:134); the cron model-selection path never did.

Fix

After all model-source overrides resolve, if cfgWithAgentDefaults.agents.defaults.agentRuntime.id is a configured CLI backend, prefer it over the model-string-derived provider. The model name itself is left untouched (the underlying model is still e.g. claude-opus-4-7); only the provider stamp changes.

Diff is +17 lines in src/cron/isolated-agent/model-selection.ts (one new import, one post-resolution check).

Repro / Impact

Local OpenClaw setup with two agents whose agentRuntime.id="claude-cli" and a cron driving each: cron session entries stamped modelProvider="anthropic" with the underlying model name, and the cron-fired turns routed through paid API at a small but persistent daily API spend that should have flowed through the user's Claude OAuth subscription. Interactive turns from the same agents routed correctly through the CLI/OAuth path.

A workspace-side detector that walks ~/.openclaw/agents/*/sessions/sessions.json, parses session keys matching agent:<id>:cron:*, and flags any whose parent agent has agentRuntime.id="claude-cli" but modelProvider != "claude-cli" confirmed three leaking sessions before the fix and zero after.

Tests

Adds 4 cases to src/cron/isolated-agent.model-formatting.test.ts under a new agentRuntime CLI provider stamping describe block:

  • positive: stamps agentRuntime.id when it is a configured CLI backend
  • positive: stamps CLI runtime even when payload.model carries a non-CLI provider prefix
  • negative: does not rewrite provider when agentRuntime.id is not a CLI backend
  • negative: leaves provider untouched when no agentRuntime is configured

Verified the new tests fail without the patch (positive cases) and pass with the patch.

Targeted lane run locally: pnpm exec node scripts/run-vitest.mjs run --config test/vitest/vitest.cron.config.ts -> 88 files / 769 tests pass. I did not run the full pnpm build && pnpm check && pnpm test lane in my dev environment; happy to add anything maintainers want.

AI assistance disclosure

  • AI-assisted (Claude Opus 4.7), fully tested at the targeted-suite level (769 cron-lane tests + 4 new regression cases).
  • I did not run codex review --base origin/main locally; will respond to any GitHub Codex / bot review conversations.

@vishutdhar
fix(cron): honor agentRuntime.id when stamping session modelProvider
1e7c1dd 
resolveCronModelSelection derived `provider` purely from the model-string
slash-prefix (e.g. "anthropic/claude-opus-4-7" -> "anthropic"), ignoring
agents.defaults.agentRuntime.id. For agents whose runtime is a CLI
backend (e.g. claude-cli), this stamped modelProvider="anthropic" on
the cron session and routed cron-fired turns through the paid Anthropic
API instead of the OAuth-backed CLI.

The interactive path already consults isCliProvider() in
run-executor.ts; this aligns the cron path with the same source of
truth. After all model-source overrides resolve, if
cfgWithAgentDefaults.agents.defaults.agentRuntime.id is a configured
CLI backend, prefer it over the model-string-derived provider.

Adds 4 regression tests under "agentRuntime CLI provider stamping"
covering positive (claude-cli stamping) and negative (non-CLI runtime,
absent runtime) paths.
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 1, 2026 •
edited
Loading

Codex review: needs changes before merge.

Summary
The PR changes cron model selection to stamp a configured CLI runtime provider for cron sessions and adds regression tests for CLI runtime provider stamping.

Reproducibility: yes. Source inspection gives a high-confidence path: configure agentRuntime.id as claude-cli with an Anthropic cron model and current main resolves provider as anthropic, while the PR also needs a mismatched OpenAI override case to avoid stamping claude-cli incorrectly.

Next step before merge
The remaining work is a narrow mechanical repair to the PR logic plus one regression test and a changelog entry.

Security
Cleared: The diff adds no dependency, workflow, secret-handling, install, or package surface; the wrong-backend behavior is a functional correctness issue.

Review findings

  • [P2] Preserve provider/runtime pairing for CLI stamping — src/cron/isolated-agent/model-selection.ts:158-164
  • [P3] Add the required changelog entry — src/cron/isolated-agent/model-selection.ts:158
Review details

Best possible solution:

Keep the fix focused on cron model selection, but reuse the existing provider/runtime compatibility rule so only compatible CLI runtimes are stamped and unrelated provider overrides remain intact.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection gives a high-confidence path: configure agentRuntime.id as claude-cli with an Anthropic cron model and current main resolves provider as anthropic, while the PR also needs a mismatched OpenAI override case to avoid stamping claude-cli incorrectly.

Is this the best way to solve the issue?

No. The PR points at the right cron selection gap, but the implementation should be provider/runtime-pair aware instead of replacing every resolved provider with any configured CLI backend id.

Full review comments:

  • [P2] Preserve provider/runtime pairing for CLI stamping — src/cron/isolated-agent/model-selection.ts:158-164
    This rewrites the resolved provider to agentRuntime.id whenever that id is a configured CLI backend. With agentRuntime.id: "claude-cli" and a cron payload selecting "openai/gpt-5.5", cron would stamp "claude-cli/gpt-5.5", while current runtime alias mapping only pairs claude-cli with Anthropic selections. Gate the rewrite through the existing provider/runtime compatibility rule and add a mismatched-provider regression test.
    Confidence: 0.88
  • [P3] Add the required changelog entry — src/cron/isolated-agent/model-selection.ts:158
    This is a user-facing cron routing and cost fix, but the PR only changes model-selection code and tests. Repo policy requires user-facing fix changes to add a CHANGELOG.md entry, so release notes would otherwise omit the bug fix.
    Confidence: 0.78

Overall correctness: patch is incorrect
Overall confidence: 0.86

Acceptance criteria:

  • pnpm test src/cron/isolated-agent.model-formatting.test.ts
  • pnpm exec oxfmt --check --threads=1 src/cron/isolated-agent/model-selection.ts src/cron/isolated-agent.model-formatting.test.ts CHANGELOG.md
  • pnpm check:changed

What I checked:

  • Docs list unavailable: The required docs list command was attempted first, but pnpm is not present in this review environment.
  • Current cron selection gap: Current main resolves provider/model from defaults, subagent config, Gmail hooks, payload model, and stored session overrides, then returns the resolved provider without consulting agentRuntime.id. (src/cron/isolated-agent/model-selection.ts:52, 15bbf4f2f304)
  • Cron executor dispatch depends on selected provider: Cron execution calls runCliAgent only when isCliProvider(providerOverride, cfgWithAgentDefaults) is true, so a resolved anthropic provider stays on the non-CLI path unless model selection stamps the CLI provider. (src/cron/isolated-agent/run-executor.ts:140, 15bbf4f2f304)
  • Interactive runtime mapping is provider/runtime pair-aware: resolveCliRuntimeExecutionProvider maps CLI runtimes by provider/runtime pair via CLI_RUNTIME_BY_PROVIDER, for example anthropic -> claude-cli, rather than treating every CLI runtime as compatible with every selected provider. (src/agents/model-runtime-aliases.ts:36, 15bbf4f2f304)
  • Interactive execution uses the pair-aware rule: The interactive attempt path routes through resolveCliRuntimeExecutionProvider before checking isCliProvider, preserving canonical provider/model selection while choosing a compatible CLI execution provider. (src/agents/command/attempt-execution.ts:400, 15bbf4f2f304)
  • Existing harness coverage preserves mismatched provider selections: Existing coverage keeps provider openai/model gpt-5.4 on PI when agentRuntime.id is claude-cli, showing claude-cli is not a valid runtime stamp for unrelated OpenAI selections. (src/agents/harness/selection.test.ts:416, 15bbf4f2f304)

Likely related people:

  • steipete: Current-main blame in the shallow checkout attributes the central cron selection, runtime-alias, interactive execution, and harness-test lines to Peter Steinberger, and the same paths define the provider/runtime behavior this PR changes. (role: recent maintainer and adjacent owner; confidence: medium; commits: 47375fd6dc98; files: src/cron/isolated-agent/model-selection.ts, src/cron/isolated-agent/run-executor.ts, src/agents/model-runtime-aliases.ts)

Remaining risk / open question:

  • The PR needs one compatibility-aware logic adjustment and an added mismatched-provider regression test before merge.
  • The user-facing cron routing fix needs a CHANGELOG.md entry before release.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 15bbf4f2f304.

@steipete
Copy link
Copy Markdown
Contributor

steipete commented May 2, 2026

@clawsweeper automerge

@clawsweeper clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label May 2, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 2, 2026 •
edited
Loading

🦞🦞
ClawSweeper automerge is enabled.

Draft PRs stay fix-only until GitHub marks them ready for review. Pause with /clawsweeper stop.

Automerge progress:

  • 2026-05-02 23:28:31 UTC review queued [`1e7c1dd22f9f`](https://github.com/openclaw/openclaw/commit/1e7c1dd22f9fc68e78d7cbc4854cb8234c6ff64d) (queued)
  • 2026-05-02 23:29:38 UTC repair queued [`1e7c1dd22f9f`](https://github.com/openclaw/openclaw/commit/1e7c1dd22f9fc68e78d7cbc4854cb8234c6ff64d) (autonomous) Run: https://github.com/openclaw/clawsweeper/actions/runs/25264571922
  • 2026-05-02 23:13:42 UTC review requested repair [`1e7c1dd22f9f`](https://github.com/openclaw/openclaw/commit/1e7c1dd22f9fc68e78d7cbc4854cb8234c6ff64d) (structured ClawSweeper marker: fix-required (finding=review-feedback sha=1e7c1d...)

@clawsweeper clawsweeper Bot mentioned this pull request May 3, 2026
Merged
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 3, 2026

ClawSweeper 🐠 reef update

Thanks for the work here. ClawSweeper could not write to the source branch, so it opened a replacement PR rather than letting the fix drift. attribution still points back here.

Replacement PR: #76319
This source PR is being closed only under the explicit source-close setting for this ClawSweeper run.
Credit follows the fix over to the replacement PR. no sneaky treasure grab.

fish notes: model gpt-5.5, reasoning high; reviewed against 02b6b6c.

@clawsweeper clawsweeper Bot closed this May 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vishutdhar @steipete