Skip to content

fix(gateway): stop Desktop retries from silently truncating session history - #86605

Closed
fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/gateway-reject-ordinal-only-truncation
Closed

fix(gateway): stop Desktop retries from silently truncating session history#86605
fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/gateway-reject-ordinal-only-truncation

Conversation

@fangliquanflq

Copy link
Copy Markdown
Contributor

What does this PR do?

Desktop retries and resubmits could silently truncate a durable session at the wrong user turn when the client supplied only a renderer ordinal. This could discard active model context while the Desktop transcript still showed those messages. The gateway now fails closed for ordinal-only truncation when durable user row IDs are available, while preserving ordinal-only compatibility for ephemeral histories without durable row IDs.

Symptom

After a failed Desktop turn, retrying or resubmitting could reduce the gateway history at an earlier user turn even though the Desktop transcript still displayed the dropped messages.

Impact

The Desktop transcript and the model's persisted context could silently diverge. In the reported session, 68 active context records were dropped from the model history until recovery from the archive.

Bug Cause

Trigger: tui_gateway/methods_prompt.py in the prompt.submit ordinal fallback when confirm_truncate is true and only truncate_before_user_ordinal is supplied.

Causal chain:

  1. A Desktop retry or resubmit supplies a renderer-visible user ordinal without a durable row ID.
  2. After history compaction or rebuild, renderer and gateway ordinals can refer to different user turns, but the gateway trusted the ordinal and persisted the selected prefix.
  3. The model loses active context from the incorrectly selected turn onward while the Desktop transcript can continue to show those messages.

Why it is wrong: Durable row IDs are stable across rebuilds, while ordinals are positional and can drift. Trusting the weaker positional target when durable identities exist permits an ambiguous destructive operation.

Working sibling / contrast: Requests with truncate_before_row_id or a message ID already resolve a durable target and reconcile the client ordinal before truncating. Ephemeral histories without durable row IDs retain the legacy ordinal-only compatibility path.

Ruled out: Failed-turn prompt planning itself does not request truncation. Explicit Desktop restore, reload, and edit paths use the shared truncation helper and attach a row ID when one is available, so the unsafe behavior was isolated to the gateway's ordinal-only fallback.

Fix

Reject ordinal-only truncation before any in-memory or database mutation when an active user turn has a durable row ID. A regression test exercises the real prompt.submit handler and verifies the request returns error 4004 without replacing persisted messages, mutating history, or starting a turn.

Related Issue

Fixes #86573

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tui_gateway/methods_prompt.py - fail closed when ordinal-only truncation targets durable session history.
  • tests/test_tui_gateway_server.py - prove the unsafe request cannot mutate memory or persistence and cannot start a turn.

How to Test

  1. Create a gateway session history whose user turns carry durable row IDs.
  2. Submit a confirmed truncation request with only truncate_before_user_ordinal.
  3. Verify error 4004 is returned and no history, persistence, or turn state changes.
scripts/run_tests.sh tests/test_tui_gateway_server.py -k test_prompt_submit_refuses_ordinal_only_when_history_has_row_ids
scripts/run_tests.sh tests/test_tui_gateway_server.py

The targeted regression passed (1 test). The related file passed all 556 tests; the wrapper retried one unrelated scheduling timeout and the fresh retry passed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run the repository test entry on the relevant gateway test file and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • Documentation updates are N/A because the public interface is unchanged
  • cli-config.yaml.example updates are N/A because no config keys changed
  • CONTRIBUTING.md and AGENTS.md updates are N/A because architecture and workflows are unchanged
  • I've considered cross-platform impact; the changed handler logic is platform-independent
  • Tool description and schema updates are N/A because no model tool behavior changed

Screenshots / Logs

N/A. The automated regression verifies the failure response and absence of state mutation directly.

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/desktop Electron desktop app (apps/desktop/*) area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #86649. Your commit was cherry-picked onto current main with your authorship preserved in git log. Clean extension of the #83785 row-id guard to the ordinal-only fallback, with exactly the right fail-closed/compat split — thanks for another reliable fix.

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

Labels

area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P1 High — major feature broken, no workaround sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop retry of a failed turn silently truncates history via ordinal-only path — row-id guard (#83785) does not cover it

3 participants