Skip to content

Preserve persisted child profile on rehydrate - #3

Open
olety wants to merge 2 commits into
review/deployed-stock11-142a9ccffrom
fix/persisted-profile-stock11
Open

Preserve persisted child profile on rehydrate#3
olety wants to merge 2 commits into
review/deployed-stock11-142a9ccffrom
fix/persisted-profile-stock11

Conversation

@olety

@olety olety commented Sep 7, 2026

Copy link
Copy Markdown
Member

Why

Passivated child sessions could restore the initial spawn model instead of their latest persisted profile.

Change

Restore the latest persisted model and thinking level. Use spawn-model metadata only when no model is saved. Preserve session identity, parent links, and global defaults.

Validation

  • Exact deployed stock .11 source: 142a9ccf61ec190aef065f696cfee2015a2bc892.
  • 226 focused and related tests passed.
  • Native workspace build passed.
  • No production activation.

This PR targets a pinned deployed-source review base. It excludes the unmerged supervision changes.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Essentials

Run ID: dce43203-acbe-4304-8f87-b229440ae9e4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@olety

olety commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

@cursor

cursor Bot commented Sep 7, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes which model and thinking level run on restore/rehydrate and add async catalog checks on the SDK path; behavior is narrower (safer) for private models but affects every resumed session.

Overview
Fixes passivated or re-opened RLM child sessions picking up the original spawn model (and ignoring thinking) instead of what was last saved in the session file.

Daemon rehydration now prefers buildSessionContext() for model and, when the branch has a thinking_level_change, passes that thinking level into createAgentSessionRuntime. Spawn metadata on the display entry is only used if no model is persisted. Model resolution already goes through canUseModel; unavailable or catalog-excluded persisted models fall through to the normal default/fallback path without mutating global settings.

SDK session restore applies the same rule: persisted models are accepted only when canUseModel succeeds (not merely hasConfiguredAuth), so configured-but-excluded private Prime models get an authorized fallback and a clear modelFallbackMessage.

Regression tests cover passivation → restore with set_model / set_thinking_level, spawn-metadata fallbacks, and SDK catalog-excluded private model behavior.

Reviewed by Cursor Bugbot for commit b9043d7. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor

cursor Bot commented Sep 7, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_a5853c39-5ff4-4019-8eb9-402e2e689682)

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Preserve persisted subagent profiles during rehydration

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Restores persisted child model and thinking level after daemon passivation.
• Uses spawn metadata only when no saved model exists.
• Covers restoration, fallbacks, identity, parent links, and unchanged global defaults.
Diagram

graph TD
  A["Passive child"] --> B["Open session"] --> C{"Saved profile?"}
  C -- Yes --> D["Persisted profile"] --> F{"Model usable?"}
  C -- No --> E["Spawn metadata"] --> F
  F -- Yes --> H["Rehydrated child"]
  F -- No --> G["Runtime defaults"] --> H
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Synchronize passive registry metadata
  • ➕ Allows rehydration to continue reading the child display entry directly.
  • ➕ Avoids rebuilding session context during restoration.
  • ➖ Duplicates the session log as a second profile source of truth.
  • ➖ Requires additional metadata for thinking levels and migration handling.
  • ➖ Can become stale when profile changes bypass registry synchronization.
2. Centralize precedence in generic runtime startup
  • ➕ Consolidates profile restoration and model fallback policy.
  • ➕ Could reduce daemon-specific restoration logic.
  • ➖ Requires a new way to express spawn-only fallback metadata.
  • ➖ Broadens a targeted daemon fix into shared startup behavior.
  • ➖ Raises regression risk for unrelated session resume paths.

Recommendation: Keep the PR’s session-log-first approach. The persisted branch is already authoritative for model and thinking changes, while the passive registry correctly remains immutable spawn metadata. Centralizing the policy could be considered later if other rehydration paths need the same spawn fallback semantics.

Files changed (3) +317 / -2

Bug fix (1) +8 / -2
daemon-mode.tsRestore persisted profiles before spawn metadata +8/-2

Restore persisted profiles before spawn metadata

• Builds the saved session context during child rehydration and prioritizes its latest model over spawn metadata. It also passes an explicitly persisted thinking level into runtime creation while preserving normal model fallback behavior.

packages/coding-agent/src/modes/daemon/daemon-mode.ts

Tests (1) +308 / -0
858-daemon-profile-rehydrate.test.tsCover daemon child profile rehydration and fallbacks +308/-0

Cover daemon child profile rehydration and fallbacks

• Adds regression coverage for restoring changed models and thinking levels after passivation, including 'xhigh' and 'off'. It also verifies spawn and global fallbacks, unavailable persisted models, stable identity and parent metadata, request behavior, and unchanged global defaults.

packages/coding-agent/test/suite/regressions/858-daemon-profile-rehydrate.test.ts

Documentation (1) +1 / -0
pr-858-profile-rehydrate.mdDocument corrected subagent profile restoration +1/-0

Document corrected subagent profile restoration

• Adds a release note describing the fix for passivated subagents restoring stale spawn profiles.

packages/coding-agent/.changes/pr-858-profile-rehydrate.md

@qodo-code-review

qodo-code-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. A restored child can use a blocked model ✓ Resolved 🐞 Bug ≡ Correctness
Description
rehydrateCompletedRlmSubagentOnce leaves rehydratedModel undefined when canUseModel rejects
the persisted model, but the downstream session initializer restores that same model using only
hasConfiguredAuth. When a configured private model is excluded from the currently authorized
catalog, the child receives it instead of the normal fallback and its subsequent model requests
cannot use the selected model.
Code

packages/coding-agent/src/modes/daemon/daemon-mode.ts[R3369-3372]

+			if (modelToRestore) {
+				const resolved = modelRegistry.find(modelToRestore.provider, modelToRestore.modelId);
				if (resolved && (await modelRegistry.canUseModel(resolved))) {
					rehydratedModel = resolved;
Evidence
The changed daemon path rejects a persisted model through canUseModel and omits the model
override, while runtime option merging preserves that absence. Session creation subsequently
restores the persisted model using only configured authentication; canUseModel proves that private
models require the additional authorized-catalog check, so a catalog-excluded model can be selected
again.

packages/coding-agent/src/modes/daemon/daemon-mode.ts[3363-3385]
packages/coding-agent/src/main.ts[708-717]
packages/coding-agent/src/core/sdk.ts[176-203]
packages/coding-agent/src/core/model-registry.ts[955-965]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A persisted private model rejected by `canUseModel` is omitted from the daemon override, but session initialization then restores it using the weaker `hasConfiguredAuth` check. Ensure a rejected persisted model reaches normal model fallback and cannot be selected again.

## Issue Context
`canUseModel` additionally checks whether private inference models appear in the authorized catalog. The generic persisted-session restoration path currently checks only configured authentication, so an absent override does not reliably represent model rejection.

## Fix Focus Areas
- packages/coding-agent/src/modes/daemon/daemon-mode.ts[3363-3385]
- packages/coding-agent/src/core/sdk.ts[176-203]
- packages/coding-agent/src/core/model-registry.ts[955-965]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 9 rules
Review mode: ⚖️ Balanced: This changes daemon session rehydration behavior and persisted model/thinking-level precedence, with meaningful runtime and fallback semantics despite focused tests, so it warrants a complete single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread packages/coding-agent/src/modes/daemon/daemon-mode.ts
@cursor

cursor Bot commented Sep 7, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_a6205fa1-c873-4220-87ea-1f1cd79e6645)

@olety

olety commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Addressed Qodo finding #1. Persisted-session restoration now awaits canUseModel, so catalog-excluded private models reach the existing authorized fallback. Added a real SDK regression and daemon fallback coverage. Seven focused tests pass. The affected native build passes. Existing stock-source validation had 226 passing tests. No production activation.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant