Skip to content

fix(session): clear stale auto fallback origins#92790

Closed
TurboTheTurtle wants to merge 2 commits into
openclaw:mainfrom
TurboTheTurtle:codex/92776-fallback-origin-repair
Closed

fix(session): clear stale auto fallback origins#92790
TurboTheTurtle wants to merge 2 commits into
openclaw:mainfrom
TurboTheTurtle:codex/92776-fallback-origin-repair

Conversation

@TurboTheTurtle

@TurboTheTurtle TurboTheTurtle commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #92776.

Verification

  • pnpm install --frozen-lockfile
  • pnpm exec oxfmt --write --threads=1 src/auto-reply/reply/get-reply.ts src/auto-reply/reply/model-selection.test.ts src/auto-reply/reply/model-selection.ts src/auto-reply/reply/stored-model-override.ts
  • node scripts/run-vitest.mjs src/auto-reply/reply/model-selection.test.ts
  • node scripts/run-vitest.mjs src/agents/agent-scope.test.ts
  • node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-execution.test.ts src/auto-reply/reply/followup-runner.test.ts
  • node scripts/run-vitest.mjs src/agents/agent-command.live-model-switch.test.ts
  • pnpm lint:core
  • pnpm tsgo:core

Real behavior proof

Behavior addressed: Auto-created fallback model pins with polluted modelOverrideFallbackOriginProvider / modelOverrideFallbackOriginModel no longer keep normal turns on the fallback indefinitely when the current configured primary differs from the stored origin.

Real environment tested: Local OpenClaw source checkout at b63a5e3df4f062be5206015c4d09673a4ae55eac on macOS with Node v24.15.0 and pnpm 11.2.2, running the production model-selection module through node --import tsx against a temporary session-store entry shaped like #92776.

Exact steps or command run after this patch:

node --import tsx --input-type=module <<'EOF'
import { createModelSelectionState } from "./src/auto-reply/reply/model-selection.ts";

const sessionKey = "agent:main:telegram:direct:real-proof";
const sessionEntry = {
  sessionId: "session-real-proof",
  updatedAt: Date.now(),
  providerOverride: "anthropic",
  modelOverride: "claude-opus-4-7",
  modelOverrideSource: "auto",
  modelOverrideFallbackOriginProvider: "anthropic",
  modelOverrideFallbackOriginModel: "claude-haiku-4-5",
};
const sessionStore = { [sessionKey]: sessionEntry };

const result = await createModelSelectionState({
  cfg: {},
  agentCfg: undefined,
  sessionEntry,
  sessionStore,
  sessionKey,
  defaultProvider: "anthropic",
  defaultModel: "claude-opus-4-8",
  primaryProvider: "anthropic",
  primaryModel: "claude-opus-4-8",
  provider: "anthropic",
  model: "claude-opus-4-7",
  hasModelDirective: false,
});

console.log(JSON.stringify({
  selected: { provider: result.provider, model: result.model },
  resetModelOverride: result.resetModelOverride,
  resetModelOverrideRef: result.resetModelOverrideRef,
  persistedEntry: {
    providerOverride: sessionStore[sessionKey].providerOverride ?? null,
    modelOverride: sessionStore[sessionKey].modelOverride ?? null,
    modelOverrideSource: sessionStore[sessionKey].modelOverrideSource ?? null,
    originProvider: sessionStore[sessionKey].modelOverrideFallbackOriginProvider ?? null,
    originModel: sessionStore[sessionKey].modelOverrideFallbackOriginModel ?? null,
  },
}, null, 2));
EOF

Evidence after fix: terminal output from that command:

{
  "selected": {
    "provider": "anthropic",
    "model": "claude-opus-4-8"
  },
  "resetModelOverride": true,
  "resetModelOverrideRef": "anthropic/claude-opus-4-7",
  "persistedEntry": {
    "providerOverride": null,
    "modelOverride": null,
    "modelOverrideSource": null,
    "originProvider": null,
    "originModel": null
  }
}

Observed result after fix: The selected model is the current primary anthropic/claude-opus-4-8, resetModelOverride is true for the stale fallback pin, and the stored override plus fallback-origin fields are cleared from the session entry.

What was not tested: I did not run a live provider-outage reproduction or call a real Anthropic account; this proof exercises the production OpenClaw model-selection path on a local checkout with the persisted session-state shape from #92776.

@openclaw-barnacle openclaw-barnacle Bot added size: S triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 13, 2026
@clawsweeper

clawsweeper Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 13, 2026, 5:46 PM ET / 21:46 UTC.

Summary
The PR adds stale auto-fallback-origin detection, skips and clears stale session model pins in auto-reply and direct agent-command selection, and adds regression tests.

PR surface: Source +133, Tests +113. Total +246 across 7 files.

Reproducibility: yes. Current main source shows a polluted-origin auto fallback entry can keep its stored model override while the primary probe returns undefined, matching the linked issue's reproduction shape.

Review metrics: 1 noteworthy metric.

  • Session-selection entry points: 2 updated. Both auto-reply and direct agent-command paths read persisted session model overrides, so maintainers should notice that the stale-origin invariant is now applied to both.

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] The PR intentionally changes persisted session cleanup for auto-created fallback pins, so maintainers should be comfortable that only auto or recovered-auto stale origins reset while user-owned overrides remain effective.
  • [P0] The supplied proof exercises the production model-selection module and the PR includes regression coverage, but it does not reproduce a live provider outage against real Anthropic credentials.

Maintainer options:

  1. Land the narrow cleanup (recommended)
    Accept the session/provider-routing risk because the patch limits cleanup to auto or recovered-auto stale origins, preserves user overrides, and covers both model-selection entry points.
  2. Ask for live outage proof
    If maintainers want more runtime confidence, ask for a redacted direct-session proof with a real provider fallback and recovery path before merge.

Next step before merge

  • No automated repair is needed; maintainers should make the compatibility-sensitive land decision and close the linked issue after merge.

Security
Cleared: The diff is limited to TypeScript session model-selection logic and tests; it does not add dependencies, workflows, secrets handling, or executable supply-chain surface.

Review details

Best possible solution:

Land the narrow cleanup that treats only auto or recovered-auto mismatched fallback-origin pins as stale across normal model-selection entry points while preserving user overrides and auth-profile validation.

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

Yes. Current main source shows a polluted-origin auto fallback entry can keep its stored model override while the primary probe returns undefined, matching the linked issue's reproduction shape.

Is this the best way to solve the issue?

Yes. The PR uses existing override reset helpers and a shared stale-origin predicate across auto-reply and direct agent-command paths, which is narrower than adding a config surface or broad migration path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR fixes a current-release session/provider-routing regression that can keep real sessions pinned to degraded fallback models indefinitely.
  • merge-risk: 🚨 session-state: The diff mutates persisted session model-override cleanup behavior for auto-created fallback pins.
  • merge-risk: 🚨 auth-provider: The affected state controls provider/model routing and can preserve or clear fallback auth-profile provenance during recovery.
  • 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 (terminal): The PR body includes after-fix terminal output from running the production model-selection module against the polluted persisted session shape and showing the current primary selected with override fields cleared.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from running the production model-selection module against the polluted persisted session shape and showing the current primary selected with override fields cleared.
Evidence reviewed

PR surface:

Source +133, Tests +113. Total +246 across 7 files.

View PR surface stats
Area Files Added Removed Net
Source 5 144 11 +133
Tests 2 114 1 +113
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 7 258 12 +246

What I checked:

  • Repository policy applied: Root AGENTS.md and src/agents/AGENTS.md were read fully; root policy treats session state, auth-provider routing, and fallback behavior as compatibility-sensitive review surfaces. (AGENTS.md:22, ae68006a8fba)
  • Current main stale-origin gap: Current main's resolveAutoFallbackPrimaryProbe returns undefined when the stored fallback origin provider/model differ from the current primary, which matches the linked issue's polluted-origin shape. (src/agents/agent-scope.ts:166, ae68006a8fba)
  • Current main still applies stored session overrides: Current main get-reply applies a stored session model override unless heartbeat or legacy-without-origin guards mark it stale; there is no normal-turn stale-origin cleanup on main. (src/auto-reply/reply/get-reply.ts:627, ae68006a8fba)
  • PR shared stale-origin predicate: The PR adds isStaleAutoFallbackOriginOverride, limited to auto or recovered-auto fallback provenance whose recorded origin key differs from the current primary key. (src/agents/agent-scope.ts:135, c5753882aa33)
  • Direct agent-command path covered: The PR checks stale auto-fallback origins after resolving the current primary, clears the persisted override through applyModelOverrideToSessionEntry when visible session effects are allowed, and skips the stale probe path for that turn. (src/agents/agent-command.ts:1420, c5753882aa33)
  • Auto-reply selection paths covered: The PR wires the stale-origin predicate into createModelSelectionState and early get-reply selection so stale session pins are reset or ignored before normal turns reuse them. (src/auto-reply/reply/model-selection.ts:210, c5753882aa33)

Likely related people:

  • steipete: Authored the merged auto model fallback transition work and heartbeat fallback cleanup touching the same persisted model-override and fallback-origin surfaces. (role: recent feature contributor; confidence: high; commits: 4b0f16d496e5, 751423299bb5; files: src/agents/agent-scope.ts, src/auto-reply/reply/model-selection.ts, src/auto-reply/reply/stored-model-override.ts)
  • neeravmakwana: Authored the merged legacy auto fallback pin cleanup that added sibling guards across agent-command, agent-scope, get-reply, and model-selection paths. (role: adjacent bugfix author; confidence: high; commits: 711ab45025a2; files: src/agents/agent-command.ts, src/agents/agent-scope.ts, src/auto-reply/reply/get-reply.ts)
  • Peter Lindsey: Local blame in the current checkout attributes the central fallback probe and model-selection blocks to a broad current-main snapshot, although deeper history points to the more specific merged PRs above. (role: current-line provenance; confidence: medium; commits: 3ec16bbad339; files: src/agents/agent-scope.ts, src/agents/agent-command.ts, src/auto-reply/reply/model-selection.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.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 13, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. labels Jun 13, 2026
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M and removed size: S labels Jun 13, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 13, 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 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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 13, 2026
@vincentkoc vincentkoc self-assigned this Jun 13, 2026
@vincentkoc

Copy link
Copy Markdown
Member

maintainer deep review found the stale-fallback bug is real, but this PR is not safe to keep extending or land.

After two repair passes, final autoreview still found four cross-surface inconsistencies: cleanup happens before unrelated read-only slash commands are dispatched; pre-dispatch harness delivery policy still reads the stale override; non-mutating/preserve-state agent-command paths can carry fallback-specific auth into the new primary; and the normal reply path reports stale-origin repair as a false allowlist rejection.

Those are symptoms of cleanup being implemented independently in multiple selection/dispatch paths. The canonical fix needs one prepared model-selection result, including delivery policy and auth semantics, carried through the whole turn before any persistent cleanup occurs. I am closing this PR rather than landing more one-sided guards. The underlying issue should remain open for that bounded owner-boundary refactor.

Focused repair proof passed (137 tests), but final autoreview correctly remains blocking.

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

Labels

agents Agent runtime and tooling merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. 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: 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

2 participants