Repository navigation
fix(desktop): isolate contexts when Server identity changes - #2291
jackie-cqz wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
| .as_ref() | ||
| .map(|server| server.server_id.as_str()), | ||
| ) | ||
| .await?; |
There was a problem hiding this comment.
[P3] Clear the cached report when the activation fence fails
When a previously checked profile is checked again and discovery changes or fails between observations, this early return leaves its previous entry in inner.reports although the active context was dropped. Clear or replace that report on failure so the subsequent state refresh cannot continue presenting the old deployment and Principal as verified.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
The post-write identity fence introduces a draft-loss regression (inline). The existing activation-report finding also reproduces at this commit.
| .map(|record| record.artifact.clone()) | ||
| .collect(); | ||
| Some(value) | ||
| if let Err(error) = identity_result { |
There was a problem hiding this comment.
[P2] Preserve the submitted draft when post-write verification disconnects
If /v1/memory/remember returns 200 but the subsequent discovery request temporarily returns 503, this new check clears active and increments the generation before returning unknown. NoteForm.save() then refreshes native state, and the [active?.generation] effect clears text and the dirty flag. The user loses the submitted text while being told to inspect an uncertain write; the retained WriteRecord contains no text to recover. I reproduced this through the native Manager and fed its actual state/receipt into MemoryWorkspace: both English and Chinese forms changed preserve me to an empty disabled textarea. The new unknown-write UI test misses this because form() returns the unchanged active state and uses a no-op onState. Keep the original draft available for explicit recovery/discard while clearing private Server results and preventing submission to a different target, and cover the native disconnect/state-refresh path.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
knqiufan
left a comment
knqiufan
left a comment
There was a problem hiding this comment.
The normal post-write disconnection recovery and cached-report fixes are present. A separate retry path still loses the original uncertain submission; see the inline P2.
| ) | ||
| return; | ||
| if (ticket !== lifetime.current || generation.current !== original) return; | ||
| onUncertainDraft?.(null); |
There was a problem hiding this comment.
[P2] Preserve the uncertain draft when a retry fails before dispatch
After a first save loses its response while identity remains valid, the original text is retained with an unknown receipt. Confirming another save clears that recovery copy here, before the retry is dispatched. If the retry's pre-dispatch discovery check returns 503, the native Manager disconnects and rejects the call without replacing the original unknown receipt. The finally state refresh then remounts the form and clears its text, leaving no copy of the original submission even though the user never confirmed discarding it. I reproduced this using a synthetic TCP fixture's actual Manager states/receipts in the App test; retaining the existing recovery draft makes that test pass. Keep the original uncertain input through a failed retry until it is explicitly discarded or safely resolved, and cover this retry/disconnect path.
Which issue or RFC does this PR close?
Refs #2101 (deployment identity portion of PR 01-B) and #2103, following RFC #1455. Both issues remain open for their remaining delivery and acceptance work.
Rationale for this change
Replacing a Server behind the same endpoint can preserve the same Principal while changing the deployment. Desktop previously rechecked only the Principal, allowing old Scope selections and late private results to cross that replacement.
What changes are included in this PR?
server_id. Recheck deployment identity after collecting connection facts, before business requests, and before returning Scope/Memory results.MemoryContext.serverIdIPC field, display the original deployment in write recovery feedback, and document behavior and evidence in both README locales and the validation/security guides.Are there any user-facing changes?
An observed deployment or Principal change disconnects the private context and requires rechecking the connection and selecting the Scope again. Unknown-write feedback identifies the original deployment and preserves the submitted input for manual recovery in the current window. Closing the window loses this recovery draft. No HTTP API, credential or persisted-profile format changes.
Identity is observed around requests. The public mutation contract has no conditional Server-identity precondition, so this cannot atomically prevent replacement between verification and dispatch or detect replacement that changes back between observations. Qualified legacy Servers have no durable deployment fence. Explicit readiness separation and local setup handoff remain follow-up work; existing anonymous readiness checks remain.
How was this change tested?
Windows 10 x64, Python 3.11.15 and Rust 1.95.0:
cargo +1.95.0 test --locked --manifest-path desktop/src-tauri/Cargo.toml; Clippy with--all-targets -- -D warnings; Rust formatting and IPC export drift: passed.aedf7beb1ecff2ea1fade6ccdd8e4bf6a298f799; wheel SHA-256:fde108a81a872a6228aad54832f2cd8f899a53bb4a7ff5c321871891fdbc5b68. This used the disposable fixture qualification with a dirty Desktop tree; executable/contract hashes and measured local round trips are recorded indesktop/VALIDATION.md. Production qualification records are unchanged.uv run --no-sync prek run -a: all file hooks passed; default Windowsty checkretains existing POSIX-only attribute diagnostics in unchanged Python files.ty check --python-platform linux: passed.AI usage statement
OpenAI Codex (GPT-6) assisted with implementation, local review, regression reproduction, testing and documentation. Commands and real-Server harnesses were executed locally; qualification limits are recorded above.