Skip to content

fix(cron): inherit default fallbacks for string agent jobs#91379

Closed
849261680 wants to merge 2 commits into
openclaw:mainfrom
849261680:fix/91362-cron-model-fallbacks
Closed

fix(cron): inherit default fallbacks for string agent jobs#91379
849261680 wants to merge 2 commits into
openclaw:mainfrom
849261680:fix/91362-cron-model-fallbacks

Conversation

@849261680

Copy link
Copy Markdown
Contributor

Summary

  • Fixes isolated cron runs where an agent's plain string model matched agents.defaults.model.primary but the run still received an empty fallback chain instead of inheriting agents.defaults.model.fallbacks.
  • Keeps the shared agent model fallback contract strict: string per-agent models and object-style { primary } configs still disable global fallbacks unless the cron prep path explicitly proves this is the default-primary shorthand case.
  • Threads a cron-local inheritance signal from raw runtime config through preflight and execution so the decision is not inferred from cfgWithAgentDefaults after cron rewrites defaults.
  • Intentionally does not change src/agents/agent-scope.ts, docs, config schema, provider routing, auth, or non-cron agent runs.

Linked context

Closes #91362

Related #91373

Was this requested by a maintainer or owner?

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: Isolated cron runs now inherit agents.defaults.model.fallbacks when the selected agent uses a plain string model equal to the configured default primary, while object-style primaries and differing string agent primaries remain strict.
  • Real environment tested: macOS arm64 local OpenClaw worktree from upstream/main at fc6400ede3, Node v24.15.0, pnpm v11.2.2.
  • Exact steps or command run after this patch:
node --import tsx - <<'EOF_NODE'
import { resolveCronFallbacksOverride } from './src/cron/isolated-agent/run-fallback-policy.ts';
const job = { id: 'cron-91362', name: 'cron 91362', schedule: { kind: 'cron', expr: '0 9 * * *', tz: 'UTC' }, sessionTarget: 'isolated', payload: { kind: 'agentTurn', message: 'summarize' }, state: {} };
const defaults = { model: { primary: 'deepseek/deepseek-v4-pro', fallbacks: ['deepseek/deepseek-v4-flash', 'moonshot/kimi-k2.6'] } };
const cfg = { agents: { defaults, list: [{ id: 'main', model: 'deepseek/deepseek-v4-pro' }] } };
const rewrittenDifferentStringCfg = { agents: { defaults: { model: { primary: 'anthropic/claude-sonnet-4-6', fallbacks: ['deepseek/deepseek-v4-flash', 'moonshot/kimi-k2.6'] } }, list: [{ id: 'main', model: 'anthropic/claude-sonnet-4-6' }] } };
const objectCfg = { agents: { defaults, list: [{ id: 'main', model: { primary: 'deepseek/deepseek-v4-pro' } }] } };
console.log(JSON.stringify({
  inheritedFallbacks: resolveCronFallbacksOverride({ cfg, agentId: 'main', job, inheritDefaultFallbacksForAgentStringModel: true }),
  rewrittenWithoutSignal: resolveCronFallbacksOverride({ cfg: rewrittenDifferentStringCfg, agentId: 'main', job }),
  objectPrimaryFallbacks: resolveCronFallbacksOverride({ cfg: objectCfg, agentId: 'main', job }),
}, null, 2));
EOF_NODE
node scripts/run-vitest.mjs src/cron/isolated-agent/run-fallback-policy.test.ts src/agents/agent-scope.test.ts src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts
pnpm tsgo:core:test
.agents/skills/autoreview/scripts/autoreview --mode local
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):
{
  "inheritedFallbacks": [
    "deepseek/deepseek-v4-flash",
    "moonshot/kimi-k2.6"
  ],
  "rewrittenWithoutSignal": [],
  "objectPrimaryFallbacks": []
}

node scripts/run-vitest.mjs src/cron/isolated-agent/run-fallback-policy.test.ts src/agents/agent-scope.test.ts src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts
Test Files  2 passed (2)
Tests  30 passed (30)
Test Files  1 passed (1)
Tests  44 passed (44)
[test] passed 2 Vitest shards

pnpm tsgo:core:test
$ node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfo
(exit 0)

.agents/skills/autoreview/scripts/autoreview --mode local
autoreview clean: no accepted/actionable findings reported
overall: patch is correct (0.79)
  • Observed result after fix: the fix(agent): agents.defaults.model.fallbacks not inherited by isolated cron sessions #91362 cron fallback shape resolves the configured default fallback list; the guard cases that ClawSweeper called out remain strict, including rewritten defaults without the cron-local inheritance signal and object-style agent primaries.
  • What was not tested: a live provider-backed scheduled cron execution against real model APIs. The proof exercises the cron fallback policy plus runCronIsolatedAgentTurn regression tests that verify the actual runWithModelFallback override passed after cron prepares cfgWithAgentDefaults.
  • Proof limitations or environment constraints: pnpm check:test-types was attempted first and timed out after 240s while still in the core test-type phase; pnpm tsgo:core:test, the touched core test-type lane, was then run directly and exited 0.
  • Before evidence (optional but encouraged): current upstream/main returns an empty cron fallback override for the reporter's string-agent/default-fallback setup, so resolveModelCandidateChain never consults agents.defaults.model.fallbacks for that isolated cron run.

Tests and validation

Which commands did you run?

  • pnpm install --frozen-lockfile
  • node scripts/run-vitest.mjs src/cron/isolated-agent/run-fallback-policy.test.ts src/agents/agent-scope.test.ts src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts
  • node --import tsx ... behavior proof shown above
  • pnpm exec oxfmt --write src/cron/isolated-agent/run-fallback-policy.ts src/cron/isolated-agent/run-fallback-policy.test.ts src/cron/isolated-agent/run.ts src/cron/isolated-agent/run-executor.ts src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts
  • git diff --check
  • pnpm tsgo:core:test
  • .agents/skills/autoreview/scripts/autoreview --mode local

What regression coverage was added or updated?

  • src/cron/isolated-agent/run-fallback-policy.test.ts covers the direct fallback policy: matching string default-primary inheritance, rewritten-default strictness without the explicit signal, object primary strictness, and differing string model strictness.
  • src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts covers the runtime preparation path, including explicit agent and implicit default-agent cron runs passing inherited fallbacks to runWithModelFallback, and differing string agent models staying strict after defaults are rewritten.
  • src/agents/agent-scope.test.ts remains unchanged and passing, proving shared string-model strictness was preserved.

What failed before this fix, if known?

If no test was added, why not?

  • Tests were added.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes. Isolated cron jobs in the narrow default-primary string shorthand case can now retry configured fallback models.

Did config, environment, or migration behavior change? (Yes/No)

No. This changes runtime fallback resolution only; no schema, env var, config migration, or docs contract change.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No.

What is the highest-risk area?

  • Accidentally changing strict model selection semantics for non-cron or intentionally strict cron agents.

How is that risk mitigated?

  • The shared agent-scope helper is untouched.
  • The inheritance decision is computed from raw cron runtime config before cfgWithAgentDefaults rewrites defaults.
  • Tests cover object-style strict primaries, differing string primaries, rewritten defaults without the explicit signal, and the actual runtime runWithModelFallback parameter.

Current review state

What is the next action?

  • Waiting for CI and ClawSweeper review on this PR.

What is still waiting on author, maintainer, CI, or external proof?

Which bot or reviewer comments were addressed?

AI-assisted: yes. I understand the code change and ran the validation above.

@openclaw-barnacle openclaw-barnacle Bot added size: M proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 8, 2026
@clawsweeper

clawsweeper Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 8, 2026, 5:51 AM ET / 09:51 UTC.

Summary
The PR adds a cron-local fallback inheritance signal so isolated cron jobs whose string agent model matches agents.defaults.model.primary can use agents.defaults.model.fallbacks, with regression coverage for strict guard cases.

PR surface: Source +27, Tests +306. Total +333 across 5 files.

Reproducibility: yes. Source inspection on current main shows string agent models return [], cron passes the defined override through, and the fallback runner skips agents.defaults.model.fallbacks; I did not run tests because this review is read-only.

Review metrics: 1 noteworthy metric.

  • Fallback policy signal: 1 added. The new optional cron-local signal decides when execution may reuse global fallback providers, so it deserves maintainer attention beyond ordinary source/test LOC.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P2] Merging intentionally changes isolated cron fallback behavior: matching string/default-primary cron jobs can now route to configured fallback providers and credentials when the primary fails.
  • [P2] This PR is the safer narrow path, but maintainers should still accept the compatibility/provider-routing semantics before landing because fallback behavior is upgrade-sensitive.

Maintainer options:

  1. Land the cron-scoped fallback repair (recommended)
    Accept the narrow compatibility/provider-routing change once required checks are green because the diff keeps stored session, payload, hook, subagent, object-primary, and differing string selections strict.
  2. Pause for fallback policy review
    If maintainers do not want matching string-agent cron jobs to inherit global fallbacks, pause this PR and make the linked issue an explicit product-policy decision instead.

Next step before merge

  • No automated repair is needed from this review; the remaining action is maintainer review of the narrow compatibility/provider-routing change and normal merge gates.

Security
Cleared: The diff changes TypeScript cron fallback resolution and tests only; I found no dependency, workflow, secret-handling, install, or supply-chain change.

Review details

Best possible solution:

Land this cron-scoped repair after required checks and maintainer review, then use it as the canonical fix for the linked issue and supersede the broader related PR if this lands.

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

Yes. Source inspection on current main shows string agent models return [], cron passes the defined override through, and the fallback runner skips agents.defaults.model.fallbacks; I did not run tests because this review is read-only.

Is this the best way to solve the issue?

Yes. The cron-local signal is the best fix shape I found because it preserves shared agent-scope strictness while allowing only the default-primary string shorthand case; the broader helper change in #91373 would change more behavior.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix live resolver output plus focused test/typecheck/autoreview output showing the claimed fallback list and strict guard cases.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix live resolver output plus focused test/typecheck/autoreview output showing the claimed fallback list and strict guard cases.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a bounded cron/model fallback bugfix with limited blast radius and no evidence of a core runtime outage.
  • merge-risk: 🚨 compatibility: The PR changes existing isolated cron retry behavior for matching string/default-primary agent configs during upgrades.
  • merge-risk: 🚨 auth-provider: The new inheritance path can route a cron run from its primary model to configured fallback providers or credentials when failover occurs.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix live resolver output plus focused test/typecheck/autoreview output showing the claimed fallback list and strict guard cases.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix live resolver output plus focused test/typecheck/autoreview output showing the claimed fallback list and strict guard cases.
Evidence reviewed

PR surface:

Source +27, Tests +306. Total +333 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 3 30 3 +27
Tests 2 306 0 +306
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 336 3 +333

What I checked:

  • Current strict string-agent fallback contract: Current main treats a non-empty string agent model as an explicit empty fallback override, which is why a string per-agent model blocks default fallback inheritance. (src/agents/agent-scope.ts:419, b75d1a0b85b0)
  • Fallback runner override contract: resolveModelCandidateChain treats any defined fallbacksOverride, including [], as replacing agents.defaults.model.fallbacks. (src/agents/model-fallback.ts:758, b75d1a0b85b0)
  • Cron model source ordering: Cron model selection records whether the effective model came from default, agent, subagent, hook, payload, or stored session state, which is the right boundary for keeping user-selected session models strict. (src/cron/isolated-agent/model-selection.ts:70, b75d1a0b85b0)
  • PR gates the new inheritance path: The PR computes inheritDefaultFallbacksForAgentStringModel after cron model selection and keeps it true only for default or agent model sources, avoiding stored session, payload, hook, and subagent selections. (src/cron/isolated-agent/run.ts:657, 4562a3850bf0)
  • PR regression coverage: The diff adds direct fallback-policy tests plus runtime forwarding tests for matching string-agent inheritance, implicit default-agent inheritance, differing string strictness, object-primary strictness, and stored cron session override strictness. (src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts:453, 4562a3850bf0)
  • Contributor proof and follow-up: The PR body supplies copied live resolver output plus focused Vitest, tsgo, diff-check, and autoreview output; the author's follow-up comment says the stored-session regression was added at head 4562a3850b. (4562a3850bf0)

Likely related people:

  • lanzhi-lee: GitHub commit metadata and local blame tie the current strict fallback helper and cron fallback policy to 58bab0c2761e. (role: recent area contributor; confidence: high; commits: 58bab0c2761e; files: src/agents/agent-scope.ts, src/cron/isolated-agent/run-fallback-policy.ts, src/cron/isolated-agent/run.ts)
  • 849261680: Beyond authoring this PR, this contributor has a recent merged cron fix touching src/cron/isolated-agent/run.ts, so they are connected to this execution path. (role: recent cron contributor; confidence: medium; commits: 4780546c124d, 4562a3850bf0; files: src/cron/isolated-agent/run.ts, src/cron/isolated-agent/run.payload-fallbacks.test.ts, src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts)
  • steipete: The current fallback-helper commit includes Peter Steinberger as co-author, and the touched area is compatibility-sensitive cron/model fallback behavior. (role: adjacent owner; confidence: medium; commits: 58bab0c2761e; files: src/agents/agent-scope.ts, src/cron/isolated-agent/run-fallback-policy.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. labels Jun 8, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 8, 2026
@849261680

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated the PR to address the prior finding on stored cron session model overrides. The inheritance flag now only stays enabled when the resolved cron model source is default or agent, so stored session, payload, hook, and subagent selections remain strict.

Local validation on the latest head 4562a3850b:

node scripts/run-vitest.mjs src/cron/isolated-agent/run-fallback-policy.test.ts src/agents/agent-scope.test.ts src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts
pnpm tsgo:core:test
git diff --check
.agents/skills/autoreview/scripts/autoreview --mode local

The new regression is in src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts and keeps a stored cron session modelOverride/providerOverride strict even when the agent string model matches agents.defaults.model.primary.

@clawsweeper

clawsweeper Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 8, 2026
@vincentkoc

Copy link
Copy Markdown
Member

Landed on main as 0911f86916b895695c02b05194543da9a2d08fac.

I cherry-picked both PR commits with attribution preserved:

  • 14430ca588b5ba4df4c3862dab2e57d17d9c5ce7 from 4b5cb68d39e3803a81f78041979dcab32e5220dc
  • 0911f86916b895695c02b05194543da9a2d08fac from 4562a3850bf0815fab0765b1e1db4256f6f1c778

Proof before push:

  • focused cron/agent Vitest: passed, 75 tests
  • changed-file oxlint: passed
  • git diff --check origin/main...HEAD: passed
  • Crabbox wrapper with Blacksmith Testbox tbx_01ktkxcb2bhnhq7zdzbh99ntz5: corepack pnpm check:changed passed
  • autoreview branch review: clean, no accepted/actionable findings

Thanks for the fix, @849261680. Closing this PR because the same change is now on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(agent): agents.defaults.model.fallbacks not inherited by isolated cron sessions

2 participants