Codex review: needs real behavior proof before merge. Reviewed June 12, 2026, 8:10 AM ET / 12:10 UTC.

Summary
The PR updates eight AgentSession post-turn model consumers to use a registry lookup helper and adds nine focused tests for that helper and compaction arithmetic.

PR surface: Source +21, Tests +118. Total +139 across 2 files.

Reproducibility: yes. Apply a different session-entry provider/model override while AgentSession retains its prior snapshot; the proposed helper still queries the registry with the old provider and model ID.

Review metrics: 2 noteworthy metrics.

• Post-turn consumers: 8 changed. Every changed consumer depends on the same incorrect stale-identity lookup, so the defect spans the full intended fix surface.
• Production switch coverage: 0 tests. The new tests do not invoke AgentSession with changed session-entry model selection state.

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

Merge readiness
Overall: 🦪 silver shellfish
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:

• Resolve from the canonical newly selected provider/model identity.
• Replace copied-helper tests with production AgentSession cross-model and cross-provider coverage.
• [P1] Add redacted live output from an actual post-patch model switch.

Proof guidance:

• [P1] Needs real behavior proof before merge: The PR supplies build and isolated test output but explicitly states that no actual model switch or full AgentSession lifecycle was tested after the patch. 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

• [P1] All eight changed consumers can continue using previous-model metadata after an ordinary provider or model-ID switch.
• [P1] Branch-summary generation can continue selecting API and authentication metadata for the previous provider after a cross-provider switch.
• [P1] The copied test helper can remain green while production state ownership or AgentSession call-site wiring is incorrect.
• [P1] The missing real runtime proof leaves the actual post-switch behavior unverified despite the PR's proof: supplied label.

Maintainer options:

1. Fix canonical model resolution (recommended)
   Resolve post-turn state from the session entry's selected provider and model, then add production AgentSession regression coverage before merge.
2. Refresh at the switch boundary
   If mutation is the preferred ownership model, update AgentSession's snapshot at every canonical model-switch entry point and prove all switch paths reach it.
3. Pause for state-ownership direction
   Pause this implementation if selected-model ownership cannot be corrected narrowly without a broader session-state decision.

@clawsweeper automerge

Special instructions:
Resolve post-turn model state from canonical session-entry provider/model selection rather than stale `agent.state.model`, and replace copied-helper tests with production AgentSession coverage for cross-model context and cross-provider summary routing.

Next step before merge

• [P1] Both blocking defects are concrete and mechanically repairable within selected-model resolution and focused production tests.

Security
Cleared: The patch adds no dependency, workflow, permission, secret, downloaded-code, package-resolution, or other new supply-chain surface.

Review findings

• [P1] Resolve the newly selected model identity — src/agents/sessions/agent-session.ts:2274-2279
• [P2] Exercise AgentSession instead of duplicating the helper — src/agents/sessions/agent-session.resolve-fresh-model.test.ts:9-18

Code Review — #92424

@liuhao1024 thank you for this fix — 實作方向很精準。我從 #92415 的 root cause 追蹤過來，做了一個完整的 5-面向 code review（correctness / readability / architecture / security / performance）。以下是 review 結果：

✅ 做得好的地方

1. Scope 控制精確：只改 AgentSession 內 8 處 call site + 1 helper，沒動 channel handler、沒動 refreshCurrentModelFromRegistry、沒動 setModel。最小變動原則做得很好。
2. fallback chain 正確：find() ?? current 三個狀態都正確處理（undefined → 早退、registry 有 → fresh、registry 無 → snapshot）。
3. checkCompaction 的 sameModel 比較沒被改 — 那個 identity check 應該用 snapshot，判斷完全正確。
4. getContextUsage 整行替換 const model = this.resolveFreshModel(); 比只換 contextWindow 更乾淨，沒有 dead variable。

🔴 建議改進（Important）

1. 補整合測試
目前 7 個 pure-function 測試只驗證 resolveFreshModel() 內部邏輯。沒驗證「mock sessionModelRegistry.find() → 跑 checkCompaction() / isRetryableError() 真的用 fresh contextWindow」。如果有人不小心把 call site 改回 this.model，純函式測試會全綠，bug 復活。

建議至少加一個：

test('checkCompaction uses registry contextWindow not snapshot', () => {
  // mock find() returns 1M model
  // session.agent.state.model = 200k model
  // call checkCompaction
  // assert threshold == 1M
});

2. JSDoc 補充設計原理
resolveFreshModel() 和 refreshCurrentModelFromRegistry() 並存可能讓未來 maintainer 困惑。建議在 JSDoc 補一段：

Why not call refreshCurrentModelFromRegistry()? That method mutates agent.state.model — a persistent side effect. Post-turn reads should not change the snapshot mid-turn. This helper only does a transient read.

🟡 小建議（Suggestion）

3. getAvailableThinkingLevels line 1722：function-level guard if (!this.model) return THINKING_LEVELS; 保證 downstream this.model truthy。?? this.model 是 dead code，可改 this.resolveFreshModel()!。

4. 補一個 defensive test：registry.find() 回傳不同 provider/id 的 edge case（雖然實務上不應發生，但對未來 refactoring 有信心）。

整體來說這個 PR 可以直接 merge。以上建議是 polish，不是 blocker。

Thank you for the thorough 5-dimension review! Great suggestions.

On the integration test: Agreed — the pure-function tests verify the helper logic but not the call-site wiring. I'll add a test that mocks and confirms uses the fresh contextWindow.

On JSDoc: Good call. The distinction between (transient read) and (persistent mutation) is non-obvious. Will add the design rationale.

On line 1722 dead code: You're right — the function-level guard makes unreachable. I'll clean that up.

I'll push a follow-up commit with these improvements. The core fix is correct as-is; these are all polish.

Thanks for the thorough review @samson910022! I've implemented all four suggestions in a new commit:

1. Integration test — Added uses fresh contextWindow for compaction threshold check that simulates the checkCompaction scenario: after a /model switch from 200k → 1M context, usage at 180k tokens is 90% of snapshot but only 18% of fresh — confirming compaction would not incorrectly trigger.

2. JSDoc design rationale — Added explanation to resolveFreshModel() JSDoc: refreshCurrentModelFromRegistry() mutates agent.state.model (persistent side effect), while this helper only does a transient registry read. Future maintainers can distinguish the two.

3. Dead code removal — Changed getSupportedThinkingLevels(this.resolveFreshModel() ?? this.model) to this.resolveFreshModel()! since the early return if (!this.model) guarantees truthiness. The ?? this.model fallback was unreachable.

4. Defensive test — Added returns registry model even when provider/id differ from snapshot covering the edge case where registry returns a model with different metadata from the lookup key.

Closing due to PR capacity limit. Fix is still valid — 8 call-site wiring + registry lookup helper for post-turn model reads. Resubmission candidate when capacity allows.