Skip to content

fix(cli): fence in-flight retractions across session re-keys and switches - #6070

Merged
liugddx merged 33 commits into
apache:mainfrom
ggbdpq:fix/tui-switch-fencing
Oct 12, 2026
Merged

liugddx merged 33 commits into
apache:mainfrom
ggbdpq:fix/tui-switch-fencing

Conversation

@ggbdpq

@ggbdpq ggbdpq commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Split from #5265 per @liugddx's scope review: the retraction/switch fencing for pre-existing plain-text races lives on its own branch, so each half can be reviewed and reverted independently.

What's here

  • settleRetractions / trackRetraction and the retraction fences keyed on the owning session
  • holdSwitchWindow across session re-keys: /session, side open/close, /rewind, and now /new
  • the detaching pre-guards on goToSession / openSideConversation / newSession
  • drain-before-re-key wrappers on runSessionSwitch / runMidTurnSwitch / rewind / openSide / closeSide
  • the side-close kept-draft abort and live-draft merge; side-open moves the editor to the side draft
  • Shift+Left stays blocked while a navigation re-key is in flight
  • new in this split: /new parks behind an in-flight retraction and holds the switch window the way /session does — the P3 from the fix(cli): stage a rewound turn's quotes into the replacement submit #5265 review of 4d7764420 (586759fc7)

The quote-staging half stays in #5265; this branch removes it, so the net diff here is fencing-only (runner + its tests + one check-tui-copy.mjs whitelist line). Roughly 19 plain-text race tests, none involving quotes. Per the review suggestion, the four "drains a retraction before X" tests and the three "blocks Shift+Left while X" tests are consolidated into two table-driven tests (assertion semantics unchanged).

Relationship to #5265

  • Known layering: the side-close abort region is split across the two PRs — the plain-text kept-draft abort here, the quote-only "staged quotes were kept" abort in fix(cli): stage a rewound turn's quotes into the replacement submit #5265. Whichever of the two merges second gets rebased so both halves survive; handled at merge time.
  • Same pattern for the check-tui-copy.mjs whitelist: each PR adds its own line.

Verification

  • pi-tui-runner 257/257 (at 4d7764420 the suite was 278 with 2 Windows-only SIGTERM fails noted by the reviewer; not reproduced locally — the 17 quote tests moved to fix(cli): stage a rewound turn's quotes into the replacement submit #5265, 9 tests consolidated into 2 table-driven ones, +1 new /new test)
  • runtime-host-session-driver 85/85, tui-copy-catalog 8/8, pi-transcript 113 pass / 3 skip
  • biome check on the changed files clean; check:asf-headers pass; check-tui-copy ok
  • TDD: drains a retraction started before /new re-keys runs red before 586759fc7 (/new executed immediately: 1 !== 0) and green after

Two test assertions were re-graded to plain-text equivalents when the quote half left: discards a retraction after a mid-turn switch now asserts the editor does not contain the restored text (instead of a status-line quote count), and serializes a mid-turn switch asserts retractedTexts.at(-1) instead of a quote payload. Happy to tighten either back up.

cc @liugddx for the scope re-check.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: GLM-5.3-Flash (ZCode) implemented the retraction/switch fencing changes and their tests under human direction and review. The affected commits carry Generated-by: GLM-5.3-Flash (ZCode) trailers, and the final squash commit should keep that trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — in-flight Shift+Left retractions are drained before session re-keys (/session, /new, side open/close, /rewind), so their text is no longer lost or applied to the wrong session.
  • No

Rewinding to a turn that carried quotes used to fail closed with
rewind_unsupported_quotes, because the TUI could only refill the
human-facing text and the replacement submit would silently drop the
turn's structured context (apache#5109).

The runtime-host driver now returns the rewound turn's QuoteRefs
verbatim and forwards quotes given to submitMessage through
turn.message.submit, whose admission already accepts them (only
session-context attachments are Host-owned). Attachments and directory
references still fail closed, since the TUI cannot re-attach files.

The TUI stages the restored quotes keyed to the branched session: the
status line carries a quotes:<n> segment while staging is live, bare
/quotes lists the staged excerpts, /quotes clear discards them, and the
first admitted submit consumes the staging while a refusal or failure
restages it for the retry.

Part of apache#5109

Generated-by: GLM-5.3-Flash (ZCode)
assert.ok the recorded submit before reading its content, per the
noUnsafeOptionalChaining lint rule.

Part of apache#5109

Generated-by: GLM-5.3-Flash (ZCode)
…nly rewinds

Two review findings on apache#5265:

A failed or blocked admission restaged the rewound quotes tagged with the
Session read at callback time, so a Session switch while the admission was
in flight attached the old quotes to the next message of the wrong
conversation. The originating Session and staging generation are now
captured at dispatch, and the restore happens only when neither moved.

A quote-only rewind refills an empty prompt, and the empty-text guards in
submitPrompt, steerRunningTurn and Alt+Enter rejected it before the quote
forwarding path could run. They now treat staged quotes as meaningful
content; a cleared plate stays truly empty and keeps refusing.

Part of apache#5109

Generated-by: GLM-5.3-Flash (ZCode)
The restage guard captured stagedGeneration before the dispatch's own
clearStagedQuotes(), which bumps the generation, so the guard compared
against a pre-clear value and a blocked or failed admission never
restored the staged quotes to the draft. Read the generation after the
clear instead, and route every staged-quote write through one setter
that bumps the generation, so a re-rewind landing while an admission is
in flight is never overwritten by the older failure's restage (apache#5109
review).

Generated-by: GLM-5.3-Flash (ZCode)
… transcript

Third-round review items on the rewind quote staging:

- A session change now clears the staged quotes outright instead of only
  hiding them while the user is elsewhere: keying alone let a silent
  resurrection re-arm the quotes on return, potentially many turns
  later. The rewind re-stages its own quotes after the switch settles.
- /quotes clear distinguishes the nothing-staged case (including quotes
  that already left on an in-flight submit) instead of always claiming
  a discard.
- A quote-only submit stored no text, so the replacement message left
  no trace in the transcript — an answer to an invisible prompt. The
  durable user entry now carries the restored-quote count and renders a
  trace line for it.
- Coverage: the blocked-disposition restage, two-quote ordering across
  the status line, /quotes listing, and the submit, and /quotes routing
  mid-turn.

Generated-by: GLM-5.3-Flash (ZCode)
# Conflicts:
#	packages/cli/src/session-driver.ts
The runtime-host PTY close-wait timeout fired on a merge head whose
runtime-host tree is identical to green upstream; the PR's delta is
 confined to packages/cli. Local pi-tui + transcript suites pass on the
merge head.
…the quote trace

Two findings from the adversarial re-review at a127b13:

- An outcome_unknown submit (or an interruption after the dispatch went
  out) resolves without a receipt, and the dispatch had already consumed
  the quote staging, so the quotes vanished silently with no restage and
  no test coverage. Restage them when the receipt is absent and surface
  a notice naming the uncertainty: admission cannot be proven either
  way, and losing the user's explicit context to an unproven outcome is
  worse than a visible duplicate ride (status line shows the restore;
  /quotes clear discards it).
- The durable user-entry trace said "restored quote(s)" for every
  message carrying quotes, but StoredMessage.quotes also carries plain
  desktop quotes (including edit-restaged ones), so any quoted message
  resurfaced in the TUI mislabeled itself as rewind-restored. Word the
  trace neutrally ("· N quote(s)").

New UnknownOutcomeSubmitDriver pins the resolve-undefined path the
previous tests only exercised through rejection.

Generated-by: GLM-5.3-Flash (ZCode)
…talog

The notice introduced for the unknown submit outcome was a visible
literal in pi-tui-runner, which check:tui-copy correctly rejects — TUI
copy must go through the localized catalog. Add quotesRestoredUnknown to
the rewind catalog (en / zh-CN / zh-TW) and reference it.

Generated-by: GLM-5.3-Flash (ZCode)
An ordinary submit that carries no quotes and an explicit /quotes clear
with nothing staged leave the staging untouched, so neither advanced the
generation — an in-flight submit's failure callback could then re-arm
its QuoteRefs onto a conversation the user had already moved past.
Advance the generation on both, making the guard airtight by
construction rather than by the reader's memory of which writes bump it.

Carries the apache#5109 review finding on the runner.

Generated-by: GLM-5.3-Flash (ZCode)
Third-round review (apache#5109): a queued or steered submit carries the staged
quotes, but taking the message back (Alt+↑ or Esc) lost them. The Host's
`queue.retract` returns the full `MessageContent` — quotes included — while
`retractQueued()` kept only the text and message ids, and `acceptRetraction`
refilled only the text. A resubmit then went out without the quotes, and a
quote-only message left an empty editor.

`MakaRetractedMessages` now carries the retracted entries' quotes and
`acceptRetraction` restages them on the current session through the
generation-guarded setter, so the restoration obeys the same veto rules as
every other staging write. Two runner tests cover one and several retracted
messages. Also drop the unused `ShellRunSnapshotResult` import.

Generated-by: GLM-5.3-Flash (ZCode)
The previous commit added the quotes field to MakaRetractedMessages but
missed this deepStrictEqual expectation.

Generated-by: GLM-5.3-Flash (ZCode)
A mid-turn `/session` can land while Alt+Up's `queue.retract` is still in
flight (switchAwayMidTurn does not wait for it). The retraction response
then restored its text and restaged its quotes under the session the driver
had moved to, where the next submit could carry the abandoned context into
the wrong conversation (apache#5109 review).

Capture the owning session before the first await and discard the response
when the driver has re-keyed; the switched-to session keeps its own empty
staging. Covered by a delayed-retract/session-switch regression.

Generated-by: GLM-5.3-Flash (ZCode)
…sion

Two gaps in the retraction fence (apache#5109 review):

- The session check ran after `retractQueued()`. A parked enqueue inside
  `settlePendingEnqueues()` let a mid-turn `/session` complete during the
  wait, so the Host call read the driver's *new* session and retracted its
  queued messages — which the mismatch fence then discarded silently. The
  fence now runs before the call: a moved-past retraction never touches the
  new session's queue.
- The interrupt path still called `driver.stop()` unconditionally, so a
  Ctrl+C aimed at the abandoned session aborted the switched-to session's
  running turn. The stop now sits inside the fence; the switch flow owns
  stopping the session it left.

Both regressions drive the retraction through a held gate while the session
switch completes: no `retractQueued` call, and no stop on the landed-on
session.

Generated-by: GLM-5.3-Flash (ZCode)
Fifth-round review (apache#5109 review): the retraction fence discarded the
retracted payload whenever the driver re-keyed mid-flight, and every switch
entry point had its own gap — the idle `/session` path, the activity-lease
wait inside `runControl`, the detach toggle, and Alt+Up's own
`settlePendingEnqueues` wait all allowed a switch to land between the Host
retraction and its response.

Serialize instead: both retraction paths register on a retraction task set,
and `switchSession`/`switchAwayMidTurn` await `settleRetractions()` at their
entry. The retracted text and quotes therefore always land in the session
they were asked for (via the existing acceptRetraction restore) before any
switch re-keys the driver; the parked-enqueue regression now fires Alt+Up
while the enqueue is held, so it exercises the real race, and a new
regression pins the switch-behind-retraction ordering.

Generated-by: GLM-5.3-Flash (ZCode)
Sixth-round review (apache#5265 review) flagged the idle `/session` path: the
retraction guard ran before `runControl`, but `runControl` parks on
`activities.acquire` before reaching the switch, and Alt+Up stays live
during that wait - a retraction started there could land after the
released lease let the switch re-key, and the switched-session fence
would discard the text and quotes the Host had already taken back.

The serialization at the switch boundary (`switchSession` awaits
`settleRetractions()` before re-keying) already closes that race on
this head and supersedes the reviewed branch's flag-based guard. Pin
the reversed order with a regression: hold the session's activity
lease, start the idle `/session` so `runControl` parks on the lease,
fire Alt+Up while it waits, then release - the retracted text and
quotes must reach the editor before the switch re-keys. With the
boundary await removed the test fails (the switch re-keys before the
retraction lands, `switch:...` then `retract-done:...`); with it, the
four focused retraction tests pass individually.

Generated-by: GLM-5.3-Flash (ZCode)
…window

Seventh-round review (apache#5265 review) flagged that the idle switch's
`settleRetractions()` drain only covers retractions asked for before the
switch starts. `driver.switchSession()` is asynchronous - it stops user
commands and opens the target Session channel before it re-keys - and
Alt+Up stays live through that whole window. A retraction pressed there
captures the old session id, and when the driver re-keys before the
retract response arrives, the switched-session fence discards the
returned text and quotes after the Host already removed the queued
entries: a real loss.

Count every runner-level switch (idle `/session`, mid-turn detach, and
rewind's branch-and-switch) in a switch window and make
`retractQueuedMessages` drop keypresses while one is open. Swallowing
the keypress loses nothing: the entries stay queued on the Host and
Alt+Up works once the switch lands, retracting from the session that is
then current.

The regression test parks the fake driver inside `switchSession`,
before its re-key, and fails on the previous head with a `queue.retract`
crossing the window (`retractCalls` 1 !== 0); it passes with the gate
and fails again with the gate removed (ablation). The four previous
focused retraction tests, the rewind in-flight tests, and the full
runner suite (241 tests, only the two known pre-existing SIGTERM
failures) pass individually.

Generated-by: GLM-5.3-Flash (ZCode)
The side-conversation open and close re-key the driver through the same
internal switchSession path as `/session` (Runtime Host openSideConversation
switches onto the forked side Session, closeSideConversation switches back
onto the parent), but neither entered the sessionSwitchesInFlight window, so
Alt+Up stayed live across the re-key. A retraction asked inside that window
removes the parent's queued entries via queue.retract while the driver is
re-keying, and the response-time session fence then discards the returned
text and quotes — the same silent input loss already fixed for `/session`,
mid-turn detach, and rewind at the current head.

Hold the same window around both switch-owning operations: the open wraps
its adopt (both the idle runControl path and the mid-turn detach path) and
the close wraps its whole body behind the existing closeSideConversation
entry points. The window swallows the keypress without loss: the entries
stay on the Host queue and Alt+Up works once the re-key lands.

Regression tests park a gated fake driver inside the open's and close's
re-key, fire Alt+Up there, and assert no queue.retract crosses (retractCalls
stays 0) while the queued entry survives untouched. Both fail on the
unfixed head (retractCalls 1 !== 0) and pass with the guard; removing only
the two new holdSwitchWindow wrappers brings both back to red.

Generated-by: GLM-5.3-Flash (ZCode)
holdSwitchWindow only blocks Alt+Up requests made inside a switch window;
a retraction already in flight when /side, a side close, or a rewind
started could still resolve after the driver re-keyed. The Host removes
the queued entries as the retraction resolves, and the response-time
session fence then discards the returned text and quotes - reachable by
queuing a follow-up, pressing Alt+Up (retraction pending), and running
the command before it settles; the reviewer reproduced the loss through
real TUI key/command paths (apache#5265 review, P2).

Copy the `/session` pattern (drain pendingRetractionTasks inside the held
window, before the driver re-key starts) into the three remaining
re-keying paths: the side open's adopt (idle runControl path and mid-turn
detach path alike), the side close's runCloseSideConversation, and the
rewind's branch-and-switch. Rewind had the same disease: every rewind
re-keys onto a fresh branch Session, so a pending retraction crosses a
session change and the branch fence discards its response exactly like
the side fences.

Tests park a gated retraction on the fake driver's Host call, start the
open / close / rewind behind it, and assert the re-key waits: retract-done
lands before the re-key and the retracted text reaches the editor. The
rewind driver mints a fresh branch id per rewind so the second rewind
crosses a session change the way the real driver does. All four tests
fail on the unfixed runner (re-key observed while the retraction is still
pending), pass with the fix, and fail again with only the three
settleRetractions calls removed.

Generated-by: GLM-5.3-Flash (ZCode)
Eighth-round review (apache#5265 review) confirmed the drains order the Host
retraction before each re-key but flagged two inline P2s in what happens
to the restored payload afterwards:

- Side open: after the drain, acceptRetraction() has already put the
  parent's recovered text into the shared editor; the open then recorded
  it as parentDraft but never swapped the editor to the side view's own
  (empty) draft, so the parent's message sat in the side editor where
  Enter would resubmit it inside the side conversation. The open now
  switches the editor to the side draft after capturing parentDraft, per
  the established per-view draft contract.
- Side close: the Ctrl+C close guard admits the close against an empty
  editor, and the drain then restores the side's queued text into it —
  which the following editor.setText(parentDraft) overwrote, losing the
  recovered message. The close now aborts with a notice when the editor
  is non-empty after the drain: closing is only admitted from an empty
  draft, so anything there came from the drain (or typing under it), and
  a later Ctrl+C clears it like any draft.

Both behaviors were red under the reviewer's own reproductions (parent
text visible in the new side editor; side text absent after close) and
green with the guards; the three updated regressions also failed with
only the guards removed. Full runner suite 245/247 - the two failures
are the known pre-existing SIGTERM tests. check:tui-copy (new notice
literal registered) and check:locale-hygiene pass.

Generated-by: GLM-5.3-Flash (ZCode)
…m dying in the switch paths

Ninth-round review (apache#5265 review) flagged three P2s and three P3s in what
happens to a drained retraction's payload after the drain:

- Quotes lost across sessions (P2): every switch path cleared the staging
  a drained retraction had just staged, and the Host already removed the
  queue entries the quotes came from - the TUI copy was the only one, so
  side open, /session, a quote-only side close, and a rewind all lost
  them. Retraction-restored quotes now ride the recovered draft text,
  wherever it goes: applySwitchResult still clears turn-staged quotes
  outright (the apache#5109 rule stands) but spares the draft-riding lane; the
  side conversation's draft slots carry quotes alongside their text
  (parked on toggle/open, staged back for the owning view on return); a
  rewind that branches without its own quotes keeps them; a rewind that
  carries its own quotes still replaces them (apache#5109 semantics, now
  visible and pinned by a test with distinct quotes per side).
- Ctrl+/ lost the recovered text (P2): the toggle captured the draft
  before waiting a pending retraction out, then wrote the stale draft
  back over what the drain had restored - side editor, parent editor,
  and Host queue all empty with no notice. The capture moved after the
  switch's drain, like the side open.
- The close window overwrote live typing (P2): a close admitted against
  an empty editor left input unlocked while its driver call was in
  flight, then setText(parentDraft) discarded whatever was typed during
  the wait. The live editor content now outranks the parked parent draft
  (kept, parent draft appended below when both exist).
- Quote-only close (P3): the abort guard only looked at editor text, so
  a retraction returning quotes with no text still closed and dropped
  the staging; staged quotes now count as a keep-open reason.
- Notice wording (P3): the kept-open notice no longer claims a retracted
  message for what may be the user's own typing - two neutral literals
  replace the old one (check:tui-copy registry updated).
- Rewind test (P3): the drain-before-rewind test minted the same quote
  on both sides, so the designed replacement could not be distinguished
  from survival; each rewind now carries a distinct quote and the
  resubmit asserts the rewound turn's quote won.

Every finding verified red under the reviewer's reproduction on the
pre-fix runner and green after; new coverage asserts what the TUI
actually submitted (driver.submittedQuotes) across the side round trip,
the /session switch, and the quote-less rewind. Full pi-tui-runner
suite: 252 pass / 2 known pre-existing SIGTERM failures. check:tui-copy,
check:locale-hygiene (vs upstream/main), check:asf-headers, and biome on
the touched files pass; merge-tree against upstream/main is clean.

Generated-by: GLM-5.3-Flash (ZCode)
The last open P2 from the ninth-round review (apache#5265 review) is an
ordering bug in the submit path's staging bookkeeping:

- A submit captured the staged quotes' draft-following provenance
  AFTER its own clear: clearStagedQuotes() resets the flag, so for
  every quote-carrying submit the capture always read false. When the
  admission then failed (or resolved without a receipt),
  restageForRetry() rebuilt the staging as ordinary turn-staged
  quotes, and the next /session or side-view switch cleared them in
  applySwitchResult() - stranding the recovered text without its
  context, the exact loss the draft-following lane exists to prevent.
  The read now happens before the dispatch's clear, so a failed
  restage re-arms the quotes in the lane they were staged in and a
  later switch carries them with the draft. The generation guard
  keeps its post-clear read: the apache#5109 restage-fence semantics are
  untouched.

New coverage pins the combination path no prior test reached: a
retraction-restored quote submit whose admission fails, then a
/session switch - the resubmit after the switch must still carry the
quotes. Red under the reviewer's ordering on the pre-fix runner,
green with the moved read, and red again with only the read moved
back (ablation). Focused quote/retraction/switch/side pattern 47/47;
full pi-tui-runner suite 253/255 with the two failures being the
known pre-existing SIGTERM tests; biome on the touched files,
check:tui-copy, and check:asf-headers pass.

Generated-by: GLM-5.3-Flash (ZCode)
The mid-turn `/session` detach set `detaching` only after awaiting the
retraction drain, while every other navigation guard (`goToSession`, the
side toggle, a mid-turn side open) reads `detaching` to refuse concurrent
switches. With a retraction held in flight, two `/session` commands (or a
command and a toggle) could both pass their guard while waiting on the
same drain and then call `driver.switchSession` concurrently: the Runtime
Host driver does not serialize those calls, so the later navigation can be
lost to the channel-generation fence, and concurrent side toggles can
apply the draft handoff twice.

Reserve the detach before the drain so the existing guard holds through
adoption, and hoist the `goToSession` guard above the idle branch: the
running Turn can end while the drain is pending, which makes the idle
path reachable mid-drain. Both new regressions drive two navigations
across one held retraction and assert a single `driver.switchSession`;
both fail on the previous head with two concurrent re-keys.

Generated-by: GLM-5.3-Flash (ZCode)
…rain window for /side and /new

Main apache#5910 removed the Alt+Up retract chord and rebound retraction to
Shift+Left, so on the merge ref a `\x1b[1;3A` send no longer retracts:
this PR's retraction tests sat waiting for a refill that never came and
the CI `test` job timed out. Point all 22 staged sends at `\x1b[1;2D`
and rename the Alt+Up comments and test titles to Shift+Left, matching
the binding apache#5910 left behind.

Merging main also completes the drain-window picture the previous commit
shored up for `/session`: with the Turn ended while a mid-turn detach is
still parked inside its retraction drain, an idle `/side` — and `/new`,
which bypasses the busy gate by design (apache#3210 review) — could pass
unguarded and re-key the driver concurrently with the in-flight switch's
own re-key. Return early on `detaching` at the top of
`openSideConversation` and `newSession`. Both new regressions drive the
second navigation across one held retraction and assert it never reaches
the driver; both fail on the previous head with the concurrent re-key.

Generated-by: GLM-5.3-Flash (ZCode)
apache#5265 mixed two themes: staging rewind/retraction quotes into the
replacement submit, and retraction/switch fencing for pre-existing
plain-text races. Keep only the fencing half here — settleRetractions,
holdSwitchWindow, the detaching guards, the side-close abort, and the
retraction-fenced interrupt — so each half can be reviewed and reverted
on its own. The quote half (QuoteRef plumbing through the driver,
protocol, and transcript, quote-only rewind, the /quotes command, and
every test asserting on quotes) stays on apache#5265.

The fencing tests move over rewritten against plain text only: their
setup no longer stages quotes through /rewind, and the four "drains a
retraction before X" and three "blocks Shift+Left while X" cases are
each consolidated into one table-driven test.
/new is the only re-keying path outside the switch window: a Shift+Left
retraction still in flight across /new was discarded after the Host had
already removed the entries — the fresh-session fence drops any response
landing past startNewSession's re-key (apache#5265 review). Wrap /new in the
same holdSwitchWindow + settleRetractions pair /session uses: the pending
retraction lands, its text reaching the editor, before the re-key, and
retractions asked inside the window are blocked until it settles.
@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Oct 11, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

I found no P0–P2 issue at 586759fc707879be75f70f893680f811f47b2e42.

The retraction drain runs before session adoption, draft capture, side-session closure, rewind, and /new. The switch window prevents a new Shift+Left retraction while a session is being re-keyed, and the origin-session checks prevent an interrupt from stopping a different session. Side-session closure preserves restored text and text typed while the drain is pending.

I ran four complete CLI test files: 463 passed, with three Windows-only skips and no failures. CLI compilation/type checking, formatting, and the copy checker passed. Removing only the new /new drain from the generated runner makes the pending-retraction test fail; restoring it passes. An additional held-/new probe confirms Shift+Left cannot start a retraction inside that switch window. These checks use the real runner with controlled terminal/driver fixtures, not a live provider or interactive desktop session. The broader root build has the inherited Desktop fixture type error described on #5265; I do not claim a clean full-repository build.

Two points remain:

  • P3 — contribution disclosure: Please add the required explicit generative-tool usage declaration to the PR description. The history includes GLM-generated commits, but the current description does not state whether tools were used and which ones. Keep the applicable commit trailers as well.
  • #5265 and this branch cannot be merged together unchanged: their exact heads conflict in pi-tui-runner.ts, its tests, and check-tui-copy.mjs. After either lands, rebase the other and retain both quote and plain-text protections, then rerun the combined side-close, draft, retraction, and session-switch tests. I have not tested a conflict-resolved combination.

The merge with current main is clean and passes the epoch-217 guard. The exact-head CI run 38148780022 is successful.

@liugddx

liugddx commented Oct 11, 2026

Copy link
Copy Markdown
Member

Reviewed exact head 586759fc7. This is the fencing half split from #5265, and it is fencing-only: the six quote hits in the diff are re-indentation of existing rewind_unsupported_quotes handling. /new now drains in-flight retractions (runNewSession calls settleRetractions() before startNewSession) and holds the switch window like /session, so a Shift+Left retraction can no longer straddle the re-key. Astro-Han's mutation (dropping the /new drain fails the pending-retraction test) and CI on this head both back that up.

One thing before I approve: please add the AI-use declaration to the PR description, as CONTRIBUTING requires (the history includes GLM-generated commits), and keep the commit trailers.

Merge order: I'd land this first, then rebase #5265 on it, keeping both its quote-only side-close guard and this branch's plain-text protections.

@ggbdpq

ggbdpq commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

@liugddx Added the AI-use declaration to the PR description (GLM-5.3-Flash (ZCode), under human direction and review), plus the template checklist. The Generated-by commit trailers are kept as they are. No code changes; head is still 586759fc7.

@liugddx liugddx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving exact head 586759fc7. Everything I checked holds:

  • Scope: the diff is fencing-only. The quote hits are re-indentation of existing rewind_unsupported_quotes handling.
  • /new: it drains in-flight retractions (runNewSession calls settleRetractions() before startNewSession) and holds the switch window like /session.
  • Disclosure: the AI-use declaration is now in the description, with the Generated-by: GLM-5.3-Flash (ZCode) trailers kept. Please make sure the squash commit carries that trailer.
  • Verification: Astro-Han found no P0–P2 at this head, and his mutation (removing the /new drain fails the pending-retraction test) shows the new test guards it. CI is green, and the branch merges cleanly with current main (6b9457f5b).

After this lands, #5265 rebases on it. Your note there matters: because each branch is "merge base minus the other half", a plain rebase silently drops the QuoteRef pipeline files without conflict markers. When I review the rebased #5265, I'll check those five files are present and that the driver's rewind_unsupported_quotes path yields to the quote-side restage.

@liugddx
liugddx merged commit 1e1fde8 into apache:main Oct 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants