Skip to content

fix(agent-session): refresh this.model from store override in getContextUsage + branch summary (#92415 gaps #2 + #3)#95353

Closed
wangmiao0668000666 wants to merge 2 commits into
openclaw:mainfrom
wangmiao0668000666:fix/92415-stale-model-context-and-branch-summary
Closed

fix(agent-session): refresh this.model from store override in getContextUsage + branch summary (#92415 gaps #2 + #3)#95353
wangmiao0668000666 wants to merge 2 commits into
openclaw:mainfrom
wangmiao0668000666:fix/92415-stale-model-context-and-branch-summary

Conversation

@wangmiao0668000666

@wangmiao0668000666 wangmiao0668000666 commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Issue #92415 reports that AgentSession.this.model is a stale snapshot after /model switches. There are 8 call sites that read this.model.{contextWindow,reasoning,thinkingLevelMap,provider} and produce wrong behavior between the /model command and the next AgentSession restart.

PR #93056 (open) covers prompt() — but two gaps remain:

  • Gap Login fails with 'WebSocket Error (socket hang up)' ECONNRESET #2 — getContextUsage() is called by TUI /status, status panels, and command status tools BETWEEN turns (after /model, before the next prompt()). It reads stale this.model.contextWindow.
  • Gap WA business, groups & office hours  #3 — summarizeAndContinue's default branch-summarizer block calls getRequiredRequestAuth(model) using this.model at line 2934. Branch summary is an independent abortable async task; if /model runs while summary is in flight, the summary uses the stale model.

This PR closes both gaps with a single helper syncModelFromStoreEntry() that intentionally complements #93056 (no name conflict, no behavior overlap — same helper signature).

Changes

  • src/agents/sessions/agent-session.ts — new private syncModelFromStoreEntry() with 6 guards (storePath present, entry exists, override present, current matches, target in registry, auth configured). Call sites at getContextUsage() entry (gap Login fails with 'WebSocket Error (socket hang up)' ECONNRESET #2) and at the default-summarizer block inside summarizeAndContinue (gap WA business, groups & office hours  #3).
  • src/agents/sessions/sdk.ts — plumb new optional storePath + sessionKey through CreateAgentSessionOptions.
  • src/agents/sessions/agent-session-services.ts — same pass-through on CreateAgentSessionFromServicesOptions for the services path.
  • src/agents/sessions/agent-session.test.ts (new, 332 lines) — 8 unit tests covering both call sites and all 6 guards.
  • scripts/repro/issue-92415-stale-model-context-usage.mts (new) — gap Login fails with 'WebSocket Error (socket hang up)' ECONNRESET #2 live proof.
  • scripts/repro/issue-92415-stale-model-branch-summary.mts (new) — gap WA business, groups & office hours  #3 live proof.

Behavior rationale

The helper reads the JSON session store path (passed via storePath), looks up the entry by sessionKey, and if providerOverride/modelOverride are set to a model the registry knows with auth, swaps this.model. The 6 guards ensure the helper is a no-op when no override is pending, when the entry is missing, when the target is unknown, or when no auth is configured — mirroring setModel()'s safety. It does not call appendModelChange or setDefaultModelProvider (those side effects are owned by setModel() for explicit user switches).

Evidence

Live proof — gap #2 (getContextUsage)

$ pnpm exec tsx scripts/repro/issue-92415-stale-model-context-usage.mts
=== Issue #92415 gap #2 — getContextUsage stale model ===
initial.contextWindow  = 4096
override.contextWindow = 200000
getContextUsage()      = 200000
PASS: getContextUsage reflects override

Live proof — gap #3 (default-summarizer sync)

$ pnpm exec tsx scripts/repro/issue-92415-stale-model-branch-summary.mts
=== Issue #92415 gap #3 — syncModelFromStoreEntry call site ===
expected override    = override-provider/override-model
observed sync models = [{"provider":"override-provider","id":"override-model"}]
PASS: sync helper picks up override model

Unit + regression test results (run on current commit head 911fede3ab)

  • node scripts/run-vitest.mjs src/agents/sessions/agent-session.test.ts → 8 passed
  • node scripts/run-vitest.mjs src/agents/sessions/sdk.test.ts → 19 passed (no regression)
  • node scripts/run-vitest.mjs src/agents/sessions/session-manager.test.ts → 45 passed (no regression)
  • node scripts/run-vitest.mjs src/agents/agent-command.live-model-switch.test.ts → 65 passed (no regression)
  • node scripts/run-tsgo.mjs → exit 0
  • pnpm build → success

What was not tested

Risk

Low. The helper is a no-op when no override is in the store entry, when the registry lacks the target, or when no auth is configured. Existing callers in getContextUsage() and summarizeAndContinue see the same this.model they did before unless an override is pending.

AI-assisted disclosure

This PR was prepared with AI assistance. Real behavior proof above was run on the current commit head (911fede3ab) by hand against the local checkout, on top of the rebased upstream openclaw/main (947c21ee5a).

🤖 Generated with Claude Code

@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts agents Agent runtime and tooling size: L triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 20, 2026
@clawsweeper

clawsweeper Bot commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 20, 2026, 11:18 AM ET / 15:18 UTC.

Summary
The PR adds an AgentSession store-entry sync helper for getContextUsage and branch-summary reads, plumbs optional storePath/sessionKey options, and adds synthetic regression tests plus repro scripts.

PR surface: Source +67, Tests +337, Other +278. Total +682 across 6 files.

Reproducibility: yes. source-level. Current main persists providerOverride/modelOverride in the session store while getContextUsage and branch-summary code read the in-memory AgentSession model snapshot.

Review metrics: 1 noteworthy metric.

  • Session-store plumbing: 2 optional fields added, 0 non-test callers pass them. The new sync path is inert unless real session construction supplies storePath and sessionKey before getContextUsage or branch summaries run.

Stored data model
Persistent data-model change detected: serialized state: src/agents/sessions/agent-session-services.ts, serialized state: src/agents/sessions/agent-session.test.ts, serialized state: src/agents/sessions/agent-session.ts, serialized state: src/agents/sessions/sdk.ts, unknown-data-model-change: src/agents/sessions/agent-session.test.ts, unknown-data-model-change: src/agents/sessions/agent-session.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #92415
Summary: This PR is a candidate fix for the canonical stale AgentSession model snapshot issue, with partial overlap from another open candidate PR.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • [P1] Pass the canonical sessions.json storePath and sessionKey through real AgentSession construction paths and add coverage that fails without those fields.
  • [P1] Add redacted real CLI, Telegram, daemon logs, terminal output, or a recording showing getContextUsage/status and branch-summary behavior after a /model switch.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR includes terminal output from synthetic repro scripts and tests, but it does not show a real OpenClaw CLI, Telegram, daemon, or embedded-session model switch after the fix. 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.

Mantis proof suggestion
A live Telegram /model flow would visibly prove whether the core session-state fix reaches the reported user path. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: switch /model in a live session and verify /status/context diagnostics use the selected model before the next prompt.

Risk before merge

  • [P1] The helper returns before doing anything when storePath is absent, and current non-test session construction paths do not supply the canonical sessions.json path.
  • [P1] The proof scripts and tests set private storePath/sessionKey fields directly, so they do not prove the real OpenClaw CLI, Telegram, daemon, or embedded-session path.
  • [P1] The branch-summary proof path does not directly exercise summarizeAndContinue, so provider/auth routing for the summary call remains unproven.

Maintainer options:

  1. Wire the real store path first (recommended)
    Pass the canonical sessions.json store path and session key through the real AgentSession construction paths, then add a regression test that fails when only the transcript JSONL path or no path is available.
  2. Require real switch proof
    After the wiring fix, require redacted CLI, Telegram, daemon logs, terminal output, or a recording showing the selected provider/model is reflected in status/context and branch-summary behavior.
  3. Defer to the canonical fix if this branch stalls
    If this PR remains partial, keep the canonical issue open and consolidate the store-sync work into a narrower maintained replacement or the overlapping open PR.

Next step before merge

  • [P1] Contributor or maintainer action is needed because the blocker combines a production wiring defect with insufficient real behavior proof; automation cannot prove the contributor's live setup for them.

Security
Cleared: No concrete supply-chain or secret-handling issue was found; the provider/auth behavior is covered as merge risk and functional review.

Review findings

  • [P1] Wire the real session store path before syncing — src/agents/sessions/agent-session.ts:2312-2315
Review details

Best possible solution:

Repair this branch or the canonical fix so AgentSession reads the canonical sessions.json selection through the real session-construction boundary, with direct getContextUsage and branch-summary coverage plus real model-switch proof.

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

Yes, source-level. Current main persists providerOverride/modelOverride in the session store while getContextUsage and branch-summary code read the in-memory AgentSession model snapshot.

Is this the best way to solve the issue?

No. The helper is a plausible shape, but this PR leaves the real session construction paths without the canonical store path and proves the behavior only by mutating private fields in tests.

Full review comments:

  • [P1] Wire the real session store path before syncing — src/agents/sessions/agent-session.ts:2312-2315
    syncModelFromStoreEntry() reads readSessionEntry(this.storePath, this.sessionKey), but the real session construction paths still do not pass storePath or sessionKey. The added tests and repro scripts set those private fields after construction, so production getContextUsage() and branch-summary calls hit this guard and never see the persisted providerOverride/modelOverride. Please plumb the canonical sessions.json store path and session key through the real callers and cover that path.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P1: The linked bug affects active model/session state after user model switches, including provider/auth routing and context decisions.
  • add merge-risk: 🚨 session-state: The PR changes how a running AgentSession refreshes in-memory model state from persisted session state, but the production store boundary is not wired.
  • add merge-risk: 🚨 auth-provider: The synced model is used before branch-summary auth lookup, so wrong or missing sync can preserve stale provider credentials or route summaries through the wrong provider.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR includes terminal output from synthetic repro scripts and tests, but it does not show a real OpenClaw CLI, Telegram, daemon, or embedded-session model switch after the fix. 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 bug affects active model/session state after user model switches, including provider/auth routing and context decisions.
  • merge-risk: 🚨 session-state: The PR changes how a running AgentSession refreshes in-memory model state from persisted session state, but the production store boundary is not wired.
  • merge-risk: 🚨 auth-provider: The synced model is used before branch-summary auth lookup, so wrong or missing sync can preserve stale provider credentials or route summaries through the wrong provider.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR includes terminal output from synthetic repro scripts and tests, but it does not show a real OpenClaw CLI, Telegram, daemon, or embedded-session model switch after the fix. 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 +67, Tests +337, Other +278. Total +682 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 3 67 0 +67
Tests 1 337 0 +337
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 2 278 0 +278
Total 6 682 0 +682

What I checked:

Likely related people:

  • kiranvk-2011: Commit 251e086 added the explicit liveModelSwitchPending detection flow that this PR depends on. (role: introduced live-switch boundary; confidence: high; commits: 251e086eacbd; files: src/agents/live-model-switch.ts, src/config/sessions/types.ts)
  • steipete: Recent history shows work on scoped live model switch state and model override/session helper behavior. (role: recent session model-switch contributor; confidence: high; commits: e4ea3c03cf02, 572dd675d8ff; files: src/sessions/model-overrides.ts, src/gateway/sessions-patch.ts)
  • jalehman: Commit c85bd45 added the session patch projection seam that owns persisted model patch projection. (role: recent session patch seam contributor; confidence: medium; commits: c85bd452846b; files: src/gateway/sessions-patch.ts)
  • vincentkoc: Current checkout blame and recent commits touch central AgentSession/session runtime code near the stale read surface. (role: recent session runtime contributor; confidence: medium; commits: 9dbf8f718ff8; files: src/agents/sessions/agent-session.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: 🚨 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 20, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 20, 2026
@wangmiao0668000666 wangmiao0668000666 mentioned this pull request Jun 20, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jun 20, 2026
Open
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

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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. scripts Repository scripts size: L status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@wangmiao0668000666