fix(session-port): restore thought-level defaults in direct CLI sessions#684
Open
itsklimov wants to merge 2 commits into
Open
fix(session-port): restore thought-level defaults in direct CLI sessions#684itsklimov wants to merge 2 commits into
itsklimov wants to merge 2 commits into
Conversation
…chema) SessionEvent::CatalogUpdated gains optional current_model/current_mode/ current_effort so a catalog push can advertise the ACTIVE selection per axis, not only the option lists (the direct-CLI paths previously pushed current_value: null for any session the user had not interactively switched — the iOfficeAI#609 regression against the iOfficeAI#574 defaults). The fields are additive and #[serde(default)]: old persisted frames still deserialize (round-trip pinned by test), and this commit stamps None at every existing construction site so it builds standalone; the follow-up commit fills them. The structured config_option_rejected reconcile signal is routed PersistTier::Ephemeral here as well — an internal pump reconcile, never display history (its emitters land in the follow-up commit).
Restores the thought-level startup semantics lost when Claude and Codex moved to the direct SessionAgentTask path in iOfficeAI#609: the legacy ACP path's seed_startup_config_preferences (iOfficeAI#574) was never ported, so a fresh chat with an assistant-default thinking level opened with no effort applied and a null current_value in the picker. - The initial effort resolves as persisted selection -> create-time config.thought_level (snapshot-wins, like mode/model). The persisted lookup accepts every legacy alias key the old ACP path stored raw (reasoning_effort/thought_level/thinking_budget/thinking; the canonical key wins), an EMPTY OR WHITESPACE-ONLY persisted value BLOCKS the default (legacy presence + trim parity), and the value is validated against the persisted-handshake preload catalog (drop-invalid; an unknown catalog stays permissive). - claude: the resolved seed rides the task runtime and is applied -- awaited -- by the FIRST send_message, strictly before the first prompt, exactly once; a manual switch invalidates a not-yet-delivered seed under a shared effort-ops gate. In-band control writes perform the turn-in-flight check and the write under the SAME stdin lock the prompt path uses (closes a TOCTOU that could land a control frame mid-turn). The optimistic effort tracker is PRE-STAMPED before the wire write and restored on a synchronous write error, so a fast rejection (a control_response arriving before dispatch returns) clears the tracker by-value instead of being overwritten by a post-write stamp. On a write error the latest-set tracker is restored to the PRIOR set it replaced (not cleared to None), so a late reject of an earlier still-pending set is still correlated and reconciled rather than lost. - codex: the seed is handed to the backend via SessionConfig.reasoning_effort and applied by a DETACHED post-open sequence (no first-turn gate): validated model reconcile (ACP clear_invalid_desired_* ported to the model/list catalog), a best-effort wait for the model settings response, then the effort settings write validated against the effective model. An invalid configured model is dropped with a corrected catalog push from BOTH the not-in-catalog and the empty/timeout branches (so a model/list arriving after the timeout cannot re-stamp the invalid seed). A failed effort settings write rolls the latest-set rpc id back to the prior value and removes its now-dead pending entry, mirroring claude so a late reject of the earlier set reconciles. - Catalog truth: CatalogUpdated carries the current model/mode/effort per the previous commit; claude always observes system/init's model; codex retains isDefault and defaultReasoningEffort from model/list and serves them as currents. A confirmed model switch (thread/settings/updated -> ConfigChanged) is followed by a re-emitted CatalogUpdated snapshot carrying the NEW model's OWN default effort, so the picker shows B's default rather than A's carried-over effort even when B also advertises it (the event pump resets the stale current on the switch). That follow-up snapshot omits the mode, so the pump PRESERVES the last-known current_mode when a CatalogUpdated carries current_mode: None (a missing field means "not carried", never "clear it"), keeping the mode confirmed by the preceding ConfigChanged. - Rejection reconcile: a backend error response for the LATEST effort set (ctl/rpc-id correlated; a superseded set's late reject is dropped whole) emits a structured config_option_rejected alongside the user-visible Notice. The event pump takes the SHARED effort-ops gate at the START of the reconcile -- the task holds that same gate across its optimistic dispatch+persist, so a reject racing the write reconciles strictly AFTER it -- then clears the optimistic highlight, removes the refused value from the persisted per-conversation selection, and re-pushes a corrected frame. Limitations (deliberate scope of this compact fix; a hardened transport-state-machine variant exists as a follow-up branch): - Config-option application is OPTIMISTIC: the set is recorded (and persisted per-conversation) immediately and corrected on an explicit backend rejection; a lost/absent response is not detected. - The codex effort seed is applied post-open with no first-turn gate -- the first codex turn may briefly run on launch defaults, matching the current upstream behaviour for model/mode. - The codex startup drain treats a JSON-RPC error on the model settings write the same as a success, so an explicit model-set error may still let the detached effort attempt run against the requested model; the backend stays authoritative and the reject signal corrects the effort. - A rejected interactive pick can briefly leave the GLOBAL cross-chat default (the assistant preference) pointing at the refused level until the next pick -- this matches the existing behaviour of the model and mode axes; the per-conversation selection is rolled back correctly. - A manual thinking-level pick racing the detached codex startup seed has no serialization gate on the wire -- last write wins (in practice the seed applies within milliseconds of open). - The write-error latest-set restore closes only the ORDERED case (set A written, set B write-errors synchronously, A rejected late). It does NOT disambiguate the case where a backend rejection is proven SIMULTANEOUSLY with that set's own synchronous write error -- which set the reject belongs to is genuinely ambiguous without a result-correlated two-phase send. That is left to the hardened transport-state-machine follow-up; here the backend stays authoritative and the next pick reconciles.
18 tasks
itsklimov
marked this pull request as ready for review
July 24, 2026 21:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CatalogUpdatedwith optional current model, mode, and effort values without changing the existing schema contractContext
#574 established persisted thought-level defaults. The direct CLI session path introduced by #609 did not apply and project the same startup default behavior. This change restores that behavior for direct CLI sessions.
Validation
cargo fmt --all -- --checkenv -u AIONUI_LOG_DIR cargo clippy -p aionui-ai-agent -p aionui-session -- -D warningsenv -u AIONUI_LOG_DIR cargo test -p aionui-ai-agent -p aionui-session(all tests passed; 11 integration tests and 2 doctests ignored by their existing annotations)