Skip to content

fix(chat): lock queued prompt actions until admission completes - #806

Merged
vastsa merged 20 commits into
vastsa:mainfrom
yuxino:fix/pending-queue-cancel
Sep 21, 2026
Merged

vastsa merged 20 commits into
vastsa:mainfrom
yuxino:fix/pending-queue-cancel

Conversation

@yuxino

@yuxino yuxino commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Removing a queued prompt before the Host finishes saving it hides its temporary row, but the message returns when saving completes. Editing during that window also restores a draft while leaving the original message queued.

Keep queue actions disabled while a prompt is being saved, with a Saving tooltip, and guard pending edits/removals in the store. Once the Host acknowledges the prompt, the existing edit, remove, reorder, and send actions become available.

queue-before-after-english-hd.mp4

The before/after recording uses the real English desktop app and Rust Host, with a controlled delay at the queue-save boundary and a local model fixture. It is 2624 × 1824; idle waits are cut and routine actions are accelerated. This demonstrates the race under controlled timing, rather than claiming it reliably occurs at normal speed.

Regression coverage exercises the real queue slice and composer: pending actions stay locked, then acknowledged removal/editing survives a queue refresh, including draft attachments. The new tests fail on the original behavior and pass with the fix. English and Chinese component, interaction, and E2E specs are synchronized. No protocol, persistence format, or migration changes.

The review follow-up integrates current main without rewriting the original commits, retains both sides of the Chinese E2E additions, and gives Send now the same Saving tooltip and accessible label as the other pending actions. Admission rejection/retry now has regression coverage. The existing 130-second Host RPC timeout already releases pending state and restores the draft; no new timeout or protocol change was needed.

Validation and candidate details
  • Passed: pnpm build:js, desktop typecheck, pnpm lint, and the complete desktop suite (2,531 passed). The 15 targeted queue/composer tests also passed; new Saving-label assertions failed on the original PR before the follow-up fix.
  • Passed: cargo build -p host-core --locked, docs locale check (79 pairs, 499 pages), git diff --check, and PR base checks against both configured origin/main and actual upstream main.
  • Real macOS Electron user path used the full renderer, preload, Main, Node runtime, and freshly built Rust Host with an isolated profile and controlled local model output/admission timing. All five actions stayed disabled while saving and unlocked after acknowledgement. Removing survived session switching; editing restored text and a PNG selected through the native file picker. Withholding admission until the existing 130-second timeout restored both text and image, and retry succeeded.
  • Tested and pushed candidate: 6c1d5e5e47c1c270575a2f85bc222211d4c13cb9; base main: 46d4ee4ee3b608d103c5dacab2fbb754bacd6eca; executable tree: ae3ae4cb4f7e581414fd7132c70ff7608e8e49ae.
  • GitHub integration candidate 7b2ad9f86895b9e319e0e108f44f998ec5894efe has the identical tree. All four checks passed for the new head: JS build/typecheck/lint/architecture/tests, Rust format/lint/tests, Docs check, and PR base. Maintainer vastsa merged the PR as 128aa50442c2be197ab27c4bc6f156d3e9308703, whose tree also matches the validated candidate.
  • The embedded recording documents the original fix. The follow-up native verification above was rerun on the final candidate tree.
  • Windows/Linux native checks and the full Rust suite were not run locally for this renderer-only follow-up. CI runs the Rust checks. Controlled timing does not establish natural race frequency or live-provider interoperability.

ZxlDragonDoctor and others added 19 commits September 21, 2026 19:20
Ignore composition key events before edit shortcuts and the search
Escape handler. Otherwise canceling an IME composition can discard
an edit or close search through event bubbling.
Check the effective binding before saving any individual shortcut change.
Restoring a default must preserve both mappings when another action uses it.

Cover disable, reassign, reset, recovery and global reset with the mounted
production component, and clarify the existing conflict-safe contract.
Pending queue rows do not yet have an actionable Host id. Removing or
editing them only changed the renderer, so admission brought the message
back and could retain work the user believed had been cancelled.

Disable pending row actions and guard the store entry points until the
Host acknowledges the queue entry. Cover the delayed-admission user path
and preserve editing and removal of acknowledged drafts.
Background run updates rerender Settings with a new error callback.
Keep that callback fresh without treating its identity as a reason to
reload the project and overwrite unsaved name and folder edits.
Preserve both IME regression coverage and the scheduled-task scenarios
added on main when resolving the English and Chinese test-plan conflict.
…r-thinking-levels

Merge the verified plugin provider thinking-level preservation fix.
`config_json.models` was decoded as one `Vec<ModelBinding>`, so a single
entry that no longer matched the schema discarded every sibling and the
provider read back as the one legacy default model derived from
`defaultModelId`. The report reproduced it by deleting one binding's
`maxTokens`: 12 stored bindings read back as 1, with nothing said about
why (issue vastsa#784).

`contextWindow` and `maxTokens` are therefore optional on the wire: an
absent key, or an explicit `0`, reads as zero and is seeded with the
generic default by the normalisation that already existed for the
explicit zero. That is the same value a record carrying only
`defaultModelId` is materialized with and the same value a plugin
manifest declaring no limits already produces, so the stored array and
the manifest now agree instead of one rejecting what the other accepts.

Each entry is then decoded on its own, so an entry that still fails to
decode costs itself rather than the array: the readable entries survive
in their stored order and the failure is reported on the host log with
the provider id, the entry's index and the reason, which makes it
locatable instead of silent. An absent `models` key, an empty array, and
an array whose every entry was unreadable all still read as the legacy
binding, so a provider stays selectable whatever its stored shape; only
the third is reported, because an empty array is a legal state and an
absent one predates bindings.

No write path changes and nothing is rewritten on disk:
`providers.update` still persists exactly the bindings the client sends.

Spec §2, decisions-log D610 and the E2E scenario are updated in both
maintained locales.
Report malformed stored provider configuration explicitly and reject model-array replacements while the stored value is degraded. This prevents a partial settings view from erasing unreadable bindings and keeps unrelated provider updates safe.
fix(host-core): protect degraded model binding updates
Fragment and History API navigation does not emit will-navigate, so the
browser kept its previous URL and loading state. Accept current main-frame
in-page events while preserving invalidated-session and stale-event guards.
Allow explicit file previews without requiring a project. Copy referenced pasted inputs into each fork so its transcript remains readable independently of the source task.

Keep scratch access scoped to the current session and remove copied inputs if fork publication fails. Cover preview navigation, bounded forks, retained context, source deletion and rollback.
…nflict

fix(shortcuts): reject conflicting individual resets
fix(projects): preserve editor drafts during background updates
fix(chat): preserve editing and search during IME Escape
…avigation

fix(browser): update chrome after same-document navigation
fix(session): keep temporary-task attachments readable after branching
@vastsa

vastsa commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Verified on my side — the race is real and this fixes it at the root:

  • The optimistic row uses a pending: id (stores/slices/queue-slice.ts:149-151) and only becomes durable in the .then at :166-176. detachQueuedPrompt (:122-134) drops only the local mirror for a pending: id, and applyQueueEntries (:103-115) re-merges the host entry, which is exactly the "message comes back" symptom; editQueuedPrompt (:201-224) had no pending guard, so it wrote a prefill while the entry stayed queued.
  • With the fix, actionsLocked = promoted || pending covers reorder/send/remove/edit and the store guards remove/edit; the pending row unlocks after the host acknowledges and on the failure path, so there is no permanent lock. Strings go through i18n.

Landing blocker: conflict with current main
GitHub reports CONFLICTING. git merge-tree shows exactly one conflict — docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md — where main's new E2E-PROVIDER-stored-binding-array… section and this PR's E2E-011f addition are both appended at end of file (the English counterpart auto-merges). The three source files merge cleanly, so please rebase onto origin/main or merge it in, keeping both E2E sections.

Two follow-ups that are not blockers: the pending row's Send now is still a plain button with the normal label and no saving tooltip (the other four actions explain themselves), and there is no timeout escape if the host never acknowledges the push. Once the branch is rebased I'll run the integration candidate (typecheck + desktop suite + the new pending-action tests) and land it.

yuxino commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review and suggestions! I’ll follow up and update the fix accordingly (´▽`ʃ♡ƪ)

Integrate the reviewed queue fix with current main while retaining both
sets of E2E scenarios. Give Send now the same saving feedback as the
other locked actions, and cover admission failure followed by retry.
@vastsa
vastsa merged commit 128aa50 into vastsa:main Sep 21, 2026
4 checks passed
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.

4 participants