Skip to content

fix(agents): use configured primary as fallback origin to prevent indefinite session pinning (#92776)#92821

Closed
liuhao1024 wants to merge 1 commit into
openclaw:mainfrom
liuhao1024:fix/session-model-pinning-origin-fields
Closed

fix(agents): use configured primary as fallback origin to prevent indefinite session pinning (#92776)#92821
liuhao1024 wants to merge 1 commit into
openclaw:mainfrom
liuhao1024:fix/session-model-pinning-origin-fields

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

What

Fix the fallback origin fields (modelOverrideFallbackOriginProvider / modelOverrideFallbackOriginModel) being recorded as the fallback candidate model instead of the configured primary, which defeats the snap-back probe and pins sessions to fallback models indefinitely.

Why

resolveFallbackSelectionOrigin() falls back to params.run.provider / params.run.model when no persisted origin exists. But params.run is selectionRun — derived from the fallback candidate — not the configured primary. On the first fallback (before autoFallbackPrimaryProbe exists), this records the wrong origin. The probe guard at agent-scope.ts:166 then checks originProvider !== primaryProvider → always true → probe never runs → clearAutoFallbackPrimaryProbeSelection() never fires → session stays pinned.

How

  • Add optional primary parameter to resolveFallbackSelectionOrigin() and applyFallbackCandidateSelectionToEntry()
  • When primary is provided and no persisted origin exists, use it as the origin instead of params.run
  • In persistFallbackCandidateSelection, always pass params.followupRun.run as the primary reference

Fixes #92776

Real behavior proof

  • Behavior addressed: Session model pinning persists indefinitely when auto-fallback origin fields are polluted with the fallback model instead of the configured primary.

  • Environment tested: macOS 26.4.1 (arm64), Node v25.8.2, OpenClaw source at 06c2bd0.

  • Steps run after the patch:

    1. Ran node scripts/run-vitest.mjs run src/agents/agent-scope.test.ts — 44 tests passed
    2. Ran node scripts/run-vitest.mjs run src/auto-reply/reply/agent-runner-execution.test.ts — 194 tests passed
    3. Ran node scripts/run-vitest.mjs run src/auto-reply/reply/model-selection.test.ts — 55 tests passed
    4. Ran pnpm build — built successfully in 78.7s
    5. Verified the fix with grep -n showing the new primary parameter flow

  • Evidence after fix:

$ grep -n 'primary.*provider.*model\|params.primary\|resolveFallbackSelectionOrigin.*primary' src/auto-reply/reply/agent-runner-execution.ts
447:  primary?: { provider: string; model: string };
471:  if (params.primary) {
472:    return { provider: params.primary.provider, model: params.primary.model };
484:  primary?: { provider: string; model: string };
497:    params.origin ?? resolveFallbackSelectionOrigin({ entry: params.entry, run: params.run, primary: params.primary });

$ node scripts/run-vitest.mjs run src/agents/agent-scope.test.ts src/auto-reply/reply/agent-runner-execution.test.ts src/auto-reply/reply/model-selection.test.ts
Test Files  3 passed (3)
Tests  293 passed (293)

$ pnpm build
✓ built in 556ms
  • Observed result after fix: All 293 related tests pass. The primary parameter flows through persistFallbackCandidateSelection → applyFallbackCandidateSelectionToEntry → resolveFallbackSelectionOrigin, ensuring the origin is always the configured primary model even on the first fallback.
  • What was not tested: Live runtime behavior with actual model fallback chains (requires running gateway + failing primary model). The test suite covers the function logic but not end-to-end fallback scenarios.

@liuhao1024
fix(agents): use configured primary as fallback origin to prevent ind…
4b525c8 
…efinite session pinning

When a model fallback occurs without an existing autoFallbackPrimaryProbe
(first fallback), resolveFallbackSelectionOrigin falls back to selectionRun
(which is the fallback candidate) instead of the configured primary. This
records the wrong origin, causing the snap-back probe guard
(originProvider !== primaryProvider) to always return early — the session
stays pinned to the fallback model indefinitely.

Fix: pass params.followupRun.run (the configured primary) as the primary
reference through applyFallbackCandidateSelectionToEntry →
resolveFallbackSelectionOrigin, so the origin is always the primary model
even on the first fallback.

Fixes openclaw#92776
@openclaw-barnacle openclaw-barnacle Bot added size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 13, 2026, 9:28 PM ET / 01:28 UTC.

Summary
The PR modifies src/auto-reply/reply/agent-runner-execution.ts to thread an optional primary provider/model into fallback origin resolution.

PR surface: Source +21. Total +21 across 1 file.

Reproducibility: yes. The source-level path is high confidence: current main stores fallback origin from the fallback-selection run, and the snap-back probe rejects persisted origin fields that no longer match the configured primary.

Review metrics: 1 noteworthy metric.

  • Persisted fallback-origin fields: 2 existing session fields affected. modelOverrideFallbackOriginProvider and modelOverrideFallbackOriginModel decide whether auto fallback can probe and clear, so upgrade behavior matters before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P2] Add a repair or migration path for existing polluted auto-fallback session entries.
  • [P2] Add regression coverage for first-time fallback origin writes and already-polluted normal-turn recovery.
  • [P2] Add redacted live runtime proof from a fallback chain or affected session showing snap-back recovery.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body has tests/build/grep only; please add redacted live fallback-chain logs or terminal output, and updating the PR body should trigger a fresh ClawSweeper review, otherwise ask a maintainer for @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P2] Existing users whose session entries were already polluted by shipped versions can remain pinned because the patch only changes future fallback writes.
  • [P1] The PR does not add regression coverage for the new primary-origin path or for already-polluted normal-turn session repair.
  • [P2] The real-behavior proof is mock/test/build output only; it does not show a live fallback chain recovering or an affected persisted session snapping back.

Maintainer options:

  1. Add an upgrade repair (recommended)
    Repair or migrate existing auto-fallback session entries whose origin fields were polluted before this PR, then keep the new writer behavior for future fallbacks.
  2. Accept future-only behavior
    Maintainers may intentionally land this as a forward-only fix, but existing affected users would still need manual session reset or a separate repair issue.
Copy recommended automerge instruction 
@clawsweeper automerge

Special instructions:
Add a narrow upgrade-safe repair for existing polluted auto-fallback session entries before landing the fallback-origin writer fix, with focused regression coverage for both already-polluted sessions and first-time fallback writes.

Next step before merge

  • [P1] A narrow automated repair can add the missing upgrade/session-state repair path and regression coverage on top of the PR branch.

Security
Cleared: The diff only changes in-process fallback/session state selection and does not add dependency, workflow, secret, or code-execution surface.

Review findings

  • [P1] Repair already-polluted fallback origins — src/auto-reply/reply/agent-runner-execution.ts:1829-1837
Review details

Best possible solution:

Land a complete fallback-origin fix that keeps the future writer change and adds a narrow upgrade/session-state repair for already-polluted auto fallback entries, with regression coverage for both paths.

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

Yes. The source-level path is high confidence: current main stores fallback origin from the fallback-selection run, and the snap-back probe rejects persisted origin fields that no longer match the configured primary.

Is this the best way to solve the issue?

No. The writer change is the right future-facing fix, but the best fix also repairs already-polluted persisted auto fallback entries from shipped versions.

Full review comments:

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 971542b7f6dc.

Label changes

Label changes:

  • add P1: The linked issue reports a real session fallback regression that can keep users pinned to degraded models across turns.
  • add merge-risk: 🚨 compatibility: The patch changes fallback persistence behavior without an upgrade repair for state written by shipped versions.
  • add merge-risk: 🚨 session-state: Merging a future-writes-only fix can leave existing persisted session state stuck in the broken auto-fallback mode.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body has tests/build/grep only; please add redacted live fallback-chain logs or terminal output, and updating the PR body should trigger a fresh ClawSweeper review, otherwise ask a maintainer for @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P1: The linked issue reports a real session fallback regression that can keep users pinned to degraded models across turns.
  • merge-risk: 🚨 session-state: Merging a future-writes-only fix can leave existing persisted session state stuck in the broken auto-fallback mode.
  • merge-risk: 🚨 compatibility: The patch changes fallback persistence behavior without an upgrade repair for state written by shipped versions.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body has tests/build/grep only; please add redacted live fallback-chain logs or terminal output, and updating the PR body should trigger a fresh ClawSweeper review, otherwise ask a maintainer for @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +21. Total +21 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 23 2 +21
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 23 2 +21

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs run src/agents/agent-scope.test.ts src/auto-reply/reply/agent-runner-execution.test.ts src/auto-reply/reply/model-selection.test.ts.

What I checked:

  • Current main fallback-origin writer: Current main resolves missing fallback origin from the run passed into fallback selection, which can be the fallback candidate rather than the configured primary. (src/auto-reply/reply/agent-runner-execution.ts:444, 971542b7f6dc)
  • Probe rejects polluted origins: The snap-back probe returns undefined when stored origin provider/model differ from the current primary, matching the linked issue's stuck-session shape. (src/agents/agent-scope.ts:166, 971542b7f6dc)
  • PR branch still preserves existing persisted origins first: The new primary fallback is only used after existing persisted origin fields are checked, so entries already polluted before this PR are not repaired by this change. (src/auto-reply/reply/agent-runner-execution.ts:452, 4b525c8f0b0a)
  • PR branch future-write change: The PR passes params.followupRun.run as primary, which helps future first-fallback writes but does not cover existing persisted polluted origin fields. (src/auto-reply/reply/agent-runner-execution.ts:1829, 4b525c8f0b0a)
  • Linked issue reports shipped polluted state: The linked issue reports already-persisted polluted auto-fallback origins in stable/beta releases and explicitly calls for a one-shot repair so existing sessions can snap back.
  • Docs contract for temporary fallback: Model failover docs say auto fallback overrides periodically probe the configured origin and clear when it recovers, so indefinite pinning is broken existing behavior. Public docs: docs/concepts/model-failover.md. (docs/concepts/model-failover.md:63, 971542b7f6dc)

Likely related people:

  • spacegeologist: Current blame for both the fallback-origin resolver and snap-back probe guard points to commit 13a079b authored as Zee Zheng / @spacegeologist. (role: introduced current snapshot behavior; confidence: medium; commits: 13a079b3f846; files: src/auto-reply/reply/agent-runner-execution.ts, src/agents/agent-scope.ts)
  • steipete: Commit 4b0f16d added the auto fallback transition/probe behavior and model failover docs that this PR is repairing. (role: recent feature contributor; confidence: high; commits: 4b0f16d496e5; files: src/agents/agent-scope.ts, docs/concepts/model-failover.md)
  • neeravmakwana: Commit 711ab45 repaired adjacent legacy auto fallback pin cleanup paths in agents and reply selection. (role: recent adjacent contributor; confidence: high; commits: 711ab45025a2; files: src/agents/agent-command.ts, src/agents/agent-scope.ts, src/auto-reply/reply/get-reply.ts)
  • yu-xin-c: Commit 9974641 recently touched compatible CLI session runtime pinning in the same reply execution file. (role: recent area contributor; confidence: medium; commits: 9974641d1ee1; files: src/auto-reply/reply/agent-runner-execution.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 rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 14, 2026
@liuhao1024

liuhao1024 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Closing due to PR capacity limit. Fix uses configured primary as fallback origin for session model pinning. Resubmission candidate when capacity allows.

@liuhao1024 liuhao1024 closed this Jun 14, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 28, 2026
Closed
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

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

1 participant

@liuhao1024