fix(cron): inherit default fallbacks for string agent jobs#91379
fix(cron): inherit default fallbacks for string agent jobs#91379849261680 wants to merge 2 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 8, 2026, 5:51 AM ET / 09:51 UTC. Summary PR surface: Source +27, Tests +306. Total +333 across 5 files. Reproducibility: yes. Source inspection on current main shows string agent models return Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest 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 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 changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +27, Tests +306. Total +333 across 5 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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 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 Local validation on the latest head The new regression is in |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Landed on I cherry-picked both PR commits with attribution preserved:
Proof before push:
Thanks for the fix, @849261680. Closing this PR because the same change is now on |
Summary
agents.defaults.model.primarybut the run still received an empty fallback chain instead of inheritingagents.defaults.model.fallbacks.{ primary }configs still disable global fallbacks unless the cron prep path explicitly proves this is the default-primary shorthand case.cfgWithAgentDefaultsafter cron rewrites defaults.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)
agents.defaults.model.fallbackswhen 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.upstream/mainatfc6400ede3, Node v24.15.0, pnpm v11.2.2.runCronIsolatedAgentTurnregression tests that verify the actualrunWithModelFallbackoverride passed after cron preparescfgWithAgentDefaults.pnpm check:test-typeswas 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.upstream/mainreturns an empty cron fallback override for the reporter's string-agent/default-fallback setup, soresolveModelCandidateChainnever consultsagents.defaults.model.fallbacksfor that isolated cron run.Tests and validation
Which commands did you run?
pnpm install --frozen-lockfilenode 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.tsnode --import tsx ...behavior proof shown abovepnpm 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.tsgit diff --checkpnpm tsgo:core:test.agents/skills/autoreview/scripts/autoreview --mode localWhat regression coverage was added or updated?
src/cron/isolated-agent/run-fallback-policy.test.tscovers 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.tscovers the runtime preparation path, including explicit agent and implicit default-agent cron runs passing inherited fallbacks torunWithModelFallback, and differing string agent models staying strict after defaults are rewritten.src/agents/agent-scope.test.tsremains unchanged and passing, proving shared string-model strictness was preserved.What failed before this fix, if known?
[]for the cron fallback override when the selected agent used a plain string model, blocking fallthrough toagents.defaults.model.fallbacks.If no test was added, why not?
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?
How is that risk mitigated?
agent-scopehelper is untouched.cfgWithAgentDefaultsrewrites defaults.runWithModelFallbackparameter.Current review state
What is the next action?
What is still waiting on author, maintainer, CI, or external proof?
Which bot or reviewer comments were addressed?
resolveSelectedModelFallbacksOverride.AI-assisted: yes. I understand the code change and ran the validation above.