From 46b76ebca70b3fe011453b2ef760fc531e2887c7 Mon Sep 17 00:00:00 2001
From: Bill <billowliu2@users.noreply.github.com>
Date: Wed, 7 Oct 2026 20:44:11 +0800
Subject: [PATCH] fix(zmodem): finalize on sink errors, throttle progress, emit
 terminal events on detach

---
 src/main/zmodem.ts   | 161 +++++++++++++++++++---
 tests/zmodem-e2e.mjs | 308 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 454 insertions(+), 15 deletions(-)

diff --git a/src/main/zmodem.ts b/src/main/zmodem.ts
index 74acfc3..2846f99 100644
--- a/src/main/zmodem.ts
+++ b/src/main/zmodem.ts
@@ -29,6 +29,13 @@ import { t } from '../shared/i18n'
 const OFFER_TIMEOUT_MS = 120_000
 /** Abort a transfer that makes no byte progress for this long (ms). */
 const STALL_TIMEOUT_MS = 90_000
+/**
+ * Progress broadcast floor (ms), same value as sftp.ts. A receive calls
+ * on_input once per ZDATA subpacket (1-8KB), so an unthrottled emit is one
+ * webContents.send per packet: ~10k-100k events for a 100MB file, and every
+ * one of them copies the transfer Map in TransferPanel and re-renders it.
+ */
+const PROGRESS_THROTTLE_MS = 150
 
 const ABORT_BUFFER = Buffer.from(Zmodem.ZMLIB.ABORT_SEQUENCE)
 
@@ -62,6 +69,14 @@ interface Engine {
   transferId: string
   /** one progress terminal event at most */
   progressEmitted: boolean
+  /** Timestamp of the last *running* progress broadcast (throttle window). */
+  lastProgressAt: number
+  /**
+   * True while *we* are tearing the session down (abort). zmodem.js's abort()
+   * fires 'session_end' synchronously, and that event must not be read as the
+   * peer finishing cleanly — see makeSessionEnd().
+   */
+  ending: boolean
   doneEmitted: boolean
   offerTimer: NodeJS.Timeout | null
   stallTimer: NodeJS.Timeout | null
@@ -100,6 +115,27 @@ function emitProgress(engine: Engine, patch: Partial<TransferProgressEvent>): vo
   }
 }
 
+/**
+ * Progress broadcast with a 150ms floor (same rule as sftp.ts). `force` is for
+ * events that must land no matter what — a file boundary or the final byte
+ * count of a transfer — so the UI never stops on a stale number.
+ */
+function emitProgressThrottled(
+  engine: Engine,
+  patch: Partial<TransferProgressEvent>,
+  force = false
+): void {
+  // Nothing may report progress once the transfer is over: a late `running`
+  // event (a send loop or an accept() callback that outlives the session)
+  // would flip the renderer's finished row back to "in progress" with no
+  // terminal event left to close it.
+  if (!engine.active) return
+  const now = Date.now()
+  if (!force && now - engine.lastProgressAt < PROGRESS_THROTTLE_MS) return
+  engine.lastProgressAt = now
+  emitProgress(engine, patch)
+}
+
 function emitDone(engine: Engine, ok: boolean, message?: string): void {
   if (engine.doneEmitted) return
   engine.doneEmitted = true
@@ -122,14 +158,22 @@ function finalize(
   failState: 'cancelled' | 'error' = 'error'
 ): void {
   engine.active = false
+  // Everything below is us ending the session, not the peer: abort() fires
+  // 'session_end' synchronously (see makeSessionEnd), and that must not be
+  // mistaken for a clean finish.
+  engine.ending = true
   cancelTimers(engine)
-  if (engine.receiveStream) {
+  const stream = engine.receiveStream
+  engine.receiveStream = null
+  // Clear the slot *before* end(): an errored/destroyed sink must not be ended
+  // again (ERR_STREAM_DESTROYED), and the 'error' handler must no longer see
+  // this stream as the current one.
+  if (stream && !stream.destroyed && !stream.writableEnded) {
     try {
-      engine.receiveStream.end()
+      stream.end()
     } catch {
       // already closed
     }
-    engine.receiveStream = null
   }
   if (!ok && engine.session) {
     // Failure paths (stall watchdog, corrupt frame) must stop the peer too:
@@ -153,9 +197,20 @@ function finalize(
   else emitDone(engine, false, message ?? t('main.zmodem.transferFailed'))
 }
 
-/** Called by the zsession's own 'session_end' event (its `this` is the session). */
+/**
+ * Called by the zsession's own 'session_end' event (its `this` is the session).
+ *
+ * 'session_end' is the clean-finish signal ONLY when the peer ended it. Our own
+ * session.abort() fires it synchronously too, and letting that through re-enters
+ * finalize(ok=true) *before* the failing caller emits its own terminal events —
+ * which reported every stalled / cancelled / failed transfer as a success
+ * (progress state 'done', ZMODEM_DONE ok:true, no error message).
+ */
 function makeSessionEnd(engine: Engine): () => void {
-  return () => finalize(engine, true)
+  return () => {
+    if (engine.ending) return
+    finalize(engine, true)
+  }
 }
 
 function touchActivity(engine: Engine): void {
@@ -198,7 +253,13 @@ async function sendOneFile(
     mode: undefined
   })
   if (!xfer) return // receiver skipped the file
-  emitProgress(engine, { file: file.name, bytes: engine.bytes, totalBytes: engine.totalBytes })
+  // Forced: a file boundary must always be visible even if the previous chunk
+  // just consumed the throttle window.
+  emitProgressThrottled(
+    engine,
+    { file: file.name, bytes: engine.bytes, totalBytes: engine.totalBytes },
+    true
+  )
 
   await new Promise<void>((resolve, reject) => {
     const rs = createReadStream(file.path)
@@ -213,7 +274,12 @@ async function sendOneFile(
         return
       }
       engine.bytes += buf.length
-      emitProgress(engine, { file: file.name, bytes: engine.bytes, totalBytes: engine.totalBytes })
+      emitProgressThrottled(engine, {
+        file: file.name,
+        bytes: engine.bytes,
+        totalBytes: engine.totalBytes
+      })
+      // Not throttled: the stall watchdog measures byte progress, not UI events.
       touchActivity(engine)
       rs.resume()
     })
@@ -239,10 +305,20 @@ async function runSend(engine: Engine, paths: string[]): Promise<void> {
     engine.totalBytes = files.reduce((acc, f) => acc + f.size, 0)
 
     for (const f of files) {
-      emitProgress(engine, { file: f.name, bytes: engine.bytes, totalBytes: engine.totalBytes })
+      emitProgressThrottled(
+        engine,
+        { file: f.name, bytes: engine.bytes, totalBytes: engine.totalBytes },
+        true
+      )
       await sendOneFile(engine, session, f)
     }
-    emitProgress(engine, { file: '', bytes: engine.totalBytes, totalBytes: engine.totalBytes })
+    // Forced: the last running progress must show the true total before the
+    // terminal event lands.
+    emitProgressThrottled(
+      engine,
+      { file: '', bytes: engine.totalBytes, totalBytes: engine.totalBytes },
+      true
+    )
     // Final state + ZMODEM_DONE come from the session_end event fired by close().
     await session.close()
   } catch (err) {
@@ -264,6 +340,21 @@ function handleOffer(engine: Engine, offer: Zmodem.Offer): void {
   engine.receiveStream = stream
   emitProgress(engine, { file: name, bytes: engine.bytes, totalBytes: engine.totalBytes })
 
+  // A dead sink (ENOSPC, EACCES, path deleted mid-transfer) must end the
+  // transfer right here. Without this listener the 'error' event is unhandled
+  // and takes the process down; even with a listener, skipping finalize leaves
+  // the engine `active` — terminal output stays suppressed and keystrokes keep
+  // being swallowed until the 90s stall watchdog finally fires.
+  stream.on('error', (err: Error) => {
+    // Only the stream the engine still owns may end the transfer: a late error
+    // from a file a later offer already replaced (or from a finished transfer)
+    // must not tear down the new one. It is still *handled* either way — an
+    // unhandled 'error' is fatal.
+    if (engine.receiveStream !== stream) return
+    engine.receiveStream = null
+    finalize(engine, false, err instanceof Error ? err.message : t('main.zmodem.receiveFailed'))
+  })
+
   offer
     .accept({
       on_input: (payload: Uint8Array | number[]) => {
@@ -274,14 +365,26 @@ function handleOffer(engine: Engine, offer: Zmodem.Offer): void {
           stream.write(buf)
         }
         engine.bytes += buf.byteLength
-        emitProgress(engine, { file: name, bytes: engine.bytes, totalBytes: engine.totalBytes })
+        emitProgressThrottled(engine, {
+          file: name,
+          bytes: engine.bytes,
+          totalBytes: engine.totalBytes
+        })
+        // Not throttled: the stall watchdog measures byte progress, not UI events.
         touchActivity(engine)
       }
     })
     .then(() => {
+      // The sink may already be dead (write error, session teardown): end() on
+      // a destroyed stream raises ERR_STREAM_DESTROYED.
+      if (stream.destroyed || stream.writableEnded) return
       stream.end(() => {
         if (engine.receiveStream === stream) engine.receiveStream = null
-        emitProgress(engine, { file: name, bytes: engine.bytes, totalBytes: engine.totalBytes })
+        emitProgressThrottled(
+          engine,
+          { file: name, bytes: engine.bytes, totalBytes: engine.totalBytes },
+          true
+        )
       })
     })
     .catch((err: unknown) => {
@@ -336,6 +439,13 @@ export function attachZmodem(sessionId: string, deps: ZmodemDeps): void {
         engine.bytes = 0
         engine.totalBytes = 0
         engine.transferId = `zm-${sessionId}`
+        // 0 = window open, so the first data chunk of the new transfer is
+        // reported immediately instead of being swallowed by the previous
+        // transfer's throttle window.
+        engine.lastProgressAt = 0
+        // The previous transfer may have ended by our own abort; this one starts
+        // fresh, so its peer-driven 'session_end' must count again.
+        engine.ending = false
         emitProgress(engine, { file: '', bytes: 0, totalBytes: 0 })
         try {
           deps.broadcast(Ipc.ZMODEM_OFFER, { id: sessionId, mode: engine.mode })
@@ -376,6 +486,8 @@ export function attachZmodem(sessionId: string, deps: ZmodemDeps): void {
     confirmed: false,
     transferId: `zm-${sessionId}`,
     progressEmitted: false,
+    lastProgressAt: 0,
+    ending: false,
     doneEmitted: false,
     offerTimer: null,
     stallTimer: null,
@@ -391,6 +503,7 @@ export function attachZmodem(sessionId: string, deps: ZmodemDeps): void {
  * the CAN abort sequence directly so the remote program exits.
  */
 function abortWithoutSession(engine: Engine, message: string): void {
+  engine.ending = true
   try {
     engine.detection?.deny()
   } catch {
@@ -411,6 +524,9 @@ function abortWithoutSession(engine: Engine, message: string): void {
 
 /** Abort a confirmed session cleanly (sends CAN so remote rz/sz exits). */
 function abortSession(engine: Engine, message: string): void {
+  // Set before abort(): the abort fires 'session_end' synchronously and that
+  // must not be taken as the peer completing the transfer.
+  engine.ending = true
   try {
     engine.session?.abort()
   } catch {
@@ -498,19 +614,34 @@ export function respondZmodem(resp: ZmodemResponse): void {
   }
 }
 
-/** Detach + clean up (session close / kill). Safe to call any number of times. */
+/**
+ * Detach + clean up (session close / kill). Safe to call any number of times.
+ *
+ * A live transfer dies with the session, so it must be finalized here: nothing
+ * else will ever answer the pending offer or finish the progress row, and the
+ * renderer would keep its ZMODEM offer modal (and a `running` transfer entry)
+ * up forever. Only `active` engines are finalized — an idle engine never
+ * announced anything, and emitting a terminal event for every session close
+ * would spam the UI with failures that never happened. finalize()/emitDone()
+ * are one-shot, so a second detach (or a detach after the normal session_end)
+ * emits nothing.
+ */
 export function detachZmodem(sessionId: string): void {
   const engine = current(sessionId)
   if (!engine) return
+  if (engine.active) {
+    finalize(engine, false, t('main.zmodem.sessionClosed'), 'cancelled')
+  }
   engine.active = false
   cancelTimers(engine)
-  if (engine.receiveStream) {
+  const stream = engine.receiveStream
+  engine.receiveStream = null
+  if (stream && !stream.destroyed && !stream.writableEnded) {
     try {
-      engine.receiveStream.end()
+      stream.end()
     } catch {
       // ignore
     }
-    engine.receiveStream = null
   }
   engine.session = null
   engine.detection = null
diff --git a/tests/zmodem-e2e.mjs b/tests/zmodem-e2e.mjs
index ac20653..35c8c36 100644
--- a/tests/zmodem-e2e.mjs
+++ b/tests/zmodem-e2e.mjs
@@ -8,6 +8,16 @@
  *                      a temp dir; assert content matches + done(ok) + progress.
  *   场景 2 (upload):   our engine uploads a local temp file -> remote `rz`
  *                      receives it; assert remote content matches + done(ok).
+ *   场景 3 (throttle): slow peer, 2KB every 3ms -> the running progress
+ *                      broadcasts must stay under the 150ms floor.
+ *   场景 4 (sink err): save dir does not exist -> createWriteStream errors, the
+ *                      engine must finalize at once (not at the 90s stall) and
+ *                      release the terminal data plane.
+ *   场景 5 (detach):   session killed with an offer pending / a transfer in
+ *                      flight -> detachZmodem must emit ZMODEM_DONE + a terminal
+ *                      progress event, exactly once. Also asserts that cancelling
+ *                      a confirmed session reports ok=false, not a fake success
+ *                      (abort() fires 'session_end' synchronously).
  *
  * Pre-req (the bundle is built automatically on first run):
  *   npx esbuild src/main/zmodem.ts --bundle --platform=node --format=cjs \
@@ -102,6 +112,71 @@ async function driveRemoteSend(session, filePath) {
   await session.close()
 }
 
+/**
+ * Same as driveRemoteSend, but paced: `chunkSize` bytes every `delayMs`. Keeps
+ * the transfer running long enough for the progress throttle to be measurable
+ * (zmodem.js caps a subpacket at 8192 bytes, so an unpaced transfer is over in
+ * a few milliseconds and produces almost no events either way).
+ */
+async function driveRemoteSendPaced(session, filePath, chunkSize, delayMs) {
+  const data = readFileSync(filePath)
+  const name = basename(filePath)
+  const xfer = await session.send_offer({ name, size: data.length, mtime: new Date() })
+  if (!xfer) throw new Error('offer was skipped')
+  let off = 0
+  while (off < data.length) {
+    const end = Math.min(off + chunkSize, data.length)
+    const slice = new Uint8Array(data.buffer, data.byteOffset + off, end - off)
+    if (end < data.length) {
+      xfer.send(slice)
+      await sleep(delayMs)
+    } else {
+      await xfer.end(slice)
+    }
+    off = end
+  }
+  await session.close()
+}
+
+/** Wait for the first event on `channel` (ms budget); returns its payload. */
+async function waitForEvent(events, channel, budgetMs = 5000) {
+  const deadline = Date.now() + budgetMs
+  for (;;) {
+    const hit = events.find((e) => e.channel === channel)
+    if (hit) return hit.payload
+    if (Date.now() >= deadline) return undefined
+    await sleep(10)
+  }
+}
+
+/**
+ * Feed octets to the peer sentry. The peer's zsession throws "peer_aborted"
+ * once our engine writes its abort CAN, which is the expected outcome in the
+ * scenarios where the engine deliberately kills the transfer.
+ */
+function safeConsume(sentry, data) {
+  try {
+    sentry.consume(data)
+  } catch {
+    // peer aborted by our own CAN
+  }
+}
+
+/** Attach our engine to a fake ssh session, collecting every broadcast. */
+function attachCollecting(sessionId, writeStream) {
+  const events = []
+  engine.attachZmodem(sessionId, {
+    broadcast: (channel, payload) => events.push({ channel, payload }),
+    toTerminal: () => {},
+    writeStream
+  })
+  return events
+}
+
+const progressOf = (events) =>
+  events.filter((e) => e.channel === 'transfer:progress').map((e) => e.payload)
+const doneOf = (events) => events.filter((e) => e.channel === 'zmodem:done').map((e) => e.payload)
+
 /** Read all bytes a remote receive side has drained so far. */
 function collectRemote(self) {
   return Buffer.concat(self._incoming)
@@ -256,11 +331,244 @@ async function scenarioUpload() {
   console.log(`[upload] progress events: ${prog.length}, done(ok)=true`)
 }
 
+// ---- SCENARIO 3: progress throttle -------------------------------------------
+async function scenarioThrottle() {
+  console.log('\n=== 场景 3: 进度节流 (download, 慢速对端) ===')
+  const sessionId = `sess-${randomBytes(3).toString('hex')}`
+  const srcDir = mkTemp('.zm-src-thr-')
+  const srcFile = join(srcDir, 'paced.bin')
+  const payload = randomBytes(300_000)
+  writeFileSync(srcFile, payload)
+  const outDir = mkTemp('.zm-out-thr-')
+
+  const remote = makeRemoteSentry(
+    (detection) => {
+      const s = detection.confirm()
+      if (s.type === 'send') {
+        // 2KB per tick => ~150 on_input calls: an unthrottled engine turns
+        // every one of them into a broadcast.
+        driveRemoteSendPaced(s, srcFile, 2048, 3).catch((e) => fail(`remote send error: ${e}`))
+      }
+    },
+    (buf) => setTimeout(() => engine.feedZmodem(sessionId, buf), 0)
+  )
+
+  const events = attachCollecting(sessionId, (id, data) =>
+    setTimeout(() => safeConsume(remote.sentry, data), 0)
+  )
+  engine.feedZmodem(sessionId, Buffer.from(zmodem.Header.build('ZRQINIT').to_hex()))
+
+  const offer = await waitForEvent(events, 'zmodem:offer')
+  if (!offer) fail('throttle: no ZMODEM_OFFER received')
+
+  const t0 = Date.now()
+  engine.respondZmodem({ id: offer.id, cancelled: false, dir: outDir })
+  const done = await waitForEvent(events, 'zmodem:done')
+  const elapsed = Date.now() - t0
+  if (!done) fail('throttle: no ZMODEM_DONE received')
+  if (!done.ok) fail(`throttle: expected ok, got ${JSON.stringify(done)}`)
+
+  const got = readFileSync(join(outDir, 'paced.bin'))
+  if (got.length !== payload.length || !got.equals(payload)) {
+    fail(`throttle: content mismatch (len ${got.length} vs ${payload.length})`)
+  }
+
+  const prog = progressOf(events)
+  // 150ms floor => at most one event per window, plus the deliberately forced
+  // file-boundary/final events (4) and slack for timer jitter. Without the
+  // throttle this is one event per subpacket (~150), so the gap is wide.
+  const bound = Math.ceil(elapsed / 150) + 8
+  if (prog.length > bound) {
+    fail(`throttle: ${prog.length} progress events in ${elapsed}ms exceeds bound ${bound}`)
+  }
+  if (prog.length < 2) fail(`throttle: only ${prog.length} progress events, throttle too aggressive`)
+  if (prog[prog.length - 1].state !== 'done') {
+    fail(`throttle: last progress state ${prog[prog.length - 1].state}, expected done`)
+  }
+  console.log(`[throttle] ${prog.length} progress events in ${elapsed}ms (bound ${bound}), content matches`)
+}
+
+// ---- SCENARIO 4: dead write sink finalizes at once ---------------------------
+async function scenarioWriteError() {
+  console.log('\n=== 场景 4: 接收落盘失败 (目录不存在) -> 立即终结 ===')
+  const sessionId = `sess-${randomBytes(3).toString('hex')}`
+  const srcDir = mkTemp('.zm-src-err-')
+  const srcFile = join(srcDir, 'fail.bin')
+  writeFileSync(srcFile, randomBytes(120_000))
+  const outDir = mkTemp('.zm-out-err-')
+  // Never created: createWriteStream() emits ENOENT, the path a full disk or a
+  // revoked permission would take.
+  const badDir = join(outDir, 'no-such-dir')
+
+  const remote = makeRemoteSentry(
+    (detection) => {
+      const s = detection.confirm()
+      if (s.type === 'send') {
+        // Paced on purpose: an unpaced in-process transfer finishes before the
+        // fs.open ENOENT even surfaces, and the sink would only fail after the
+        // session had already ended successfully. The engine aborts as soon as
+        // the sink fails, so the peer dying here is the expected outcome, not a
+        // test failure.
+        driveRemoteSendPaced(s, srcFile, 4096, 5).catch(() => {})
+      }
+    },
+    (buf) => setTimeout(() => engine.feedZmodem(sessionId, buf), 0)
+  )
+
+  const events = attachCollecting(sessionId, (id, data) =>
+    setTimeout(() => safeConsume(remote.sentry, data), 0)
+  )
+  engine.feedZmodem(sessionId, Buffer.from(zmodem.Header.build('ZRQINIT').to_hex()))
+
+  const offer = await waitForEvent(events, 'zmodem:offer')
+  if (!offer) fail('write-error: no ZMODEM_OFFER received')
+
+  const t0 = Date.now()
+  engine.respondZmodem({ id: offer.id, cancelled: false, dir: badDir })
+  // 2s budget: the point is that this does NOT wait for the 90s stall watchdog.
+  const done = await waitForEvent(events, 'zmodem:done', 2000)
+  if (!done) fail('write-error: no ZMODEM_DONE within 2s (engine stuck active?)')
+  if (done.ok) {
+    fail(`write-error: expected ok=false, got ${JSON.stringify(done)} — sink error lost the race`)
+  }
+  if (engine.isZmodemActive(sessionId)) {
+    fail('write-error: engine still active — terminal data plane stays suppressed')
+  }
+  const prog = progressOf(events)
+  const last = prog[prog.length - 1]
+  if (last.state !== 'error') fail(`write-error: last progress state ${last.state}, expected error`)
+  console.log(`[write-error] done(ok=false) after ${Date.now() - t0}ms, state=error, engine inactive`)
+}
+
+// ---- SCENARIO 5: detach (session killed) emits the terminal events ------------
+async function scenarioDetach() {
+  console.log('\n=== 场景 5: 会话被 kill -> detach 补发终结事件 ===')
+
+  // (a) offer still pending: the renderer's modal is up and nothing will answer
+  {
+    const sessionId = `sess-${randomBytes(3).toString('hex')}`
+    const events = attachCollecting(sessionId, () => {})
+    engine.feedZmodem(sessionId, Buffer.from(zmodem.Header.build('ZRQINIT').to_hex()))
+    const offer = await waitForEvent(events, 'zmodem:offer')
+    if (!offer) fail('detach(pending): no ZMODEM_OFFER received')
+
+    engine.detachZmodem(sessionId)
+    const done = doneOf(events)
+    if (done.length !== 1) fail(`detach(pending): expected 1 ZMODEM_DONE, got ${done.length}`)
+    if (done[0].ok) fail('detach(pending): expected ok=false')
+    if (engine.isZmodemActive(sessionId)) fail('detach(pending): engine still active')
+    const prog = progressOf(events)
+    if (prog[prog.length - 1].state !== 'cancelled') {
+      fail(`detach(pending): last progress state ${prog[prog.length - 1].state}, expected cancelled`)
+    }
+    // Idempotent: the engine is already gone, a second detach emits nothing.
+    engine.detachZmodem(sessionId)
+    if (doneOf(events).length !== 1) fail('detach(pending): ZMODEM_DONE emitted twice')
+    console.log('[detach] pending offer -> done(ok=false) + cancelled progress, idempotent')
+  }
+
+  // (b) transfer in flight
+  {
+    const sessionId = `sess-${randomBytes(3).toString('hex')}`
+    const srcDir = mkTemp('.zm-src-det-')
+    const srcFile = join(srcDir, 'killed.bin')
+    writeFileSync(srcFile, randomBytes(200_000))
+    const outDir = mkTemp('.zm-out-det-')
+
+    let detached = false
+    const remote = makeRemoteSentry(
+      (detection) => {
+        const s = detection.confirm()
+        if (s.type === 'send') {
+          // Killed mid-flight on purpose: a dying peer is expected here.
+          driveRemoteSendPaced(s, srcFile, 2048, 3).catch(() => {})
+        }
+      },
+      (buf) => {
+        if (detached) return
+        setTimeout(() => engine.feedZmodem(sessionId, buf), 0)
+      }
+    )
+
+    const events = attachCollecting(sessionId, (id, data) => {
+      if (detached) return
+      setTimeout(() => remote.sentry.consume(data), 0)
+    })
+    engine.feedZmodem(sessionId, Buffer.from(zmodem.Header.build('ZRQINIT').to_hex()))
+    const offer = await waitForEvent(events, 'zmodem:offer')
+    if (!offer) fail('detach(live): no ZMODEM_OFFER received')
+
+    engine.respondZmodem({ id: offer.id, cancelled: false, dir: outDir })
+    if (!engine.isZmodemActive(sessionId)) fail('detach(live): engine not active after respond')
+
+    const mark = events.length
+    detached = true
+    engine.detachZmodem(sessionId)
+    await sleep(300)
+
+    const done = doneOf(events)
+    if (done.length !== 1) fail(`detach(live): expected 1 ZMODEM_DONE, got ${done.length}`)
+    if (done[0].ok) fail('detach(live): expected ok=false')
+    if (engine.isZmodemActive(sessionId)) fail('detach(live): engine still active')
+
+    const after = progressOf(events.slice(mark))
+    if (after.length === 0) fail('detach(live): no terminal progress emitted')
+    if (after[after.length - 1].state !== 'cancelled') {
+      fail(`detach(live): last progress state ${after[after.length - 1].state}, expected cancelled`)
+    }
+    if (after.some((p) => p.state === 'running')) {
+      fail('detach(live): progress still running after detach')
+    }
+    engine.detachZmodem(sessionId)
+    if (doneOf(events).length !== 1) fail('detach(live): ZMODEM_DONE emitted twice')
+    console.log('[detach] live transfer -> done(ok=false) + cancelled progress, exactly once')
+  }
+
+  // (c) user cancels a *confirmed* session (no files chosen). abortSession()
+  // aborts before it finalizes, and the abort fires 'session_end' synchronously:
+  // without the engine.ending guard that re-entered finalize(ok=true) and the
+  // cancel was reported as a successful transfer.
+  {
+    const sessionId = `sess-${randomBytes(3).toString('hex')}`
+    const remote = makeRemoteSentry(
+      (detection) => {
+        const s = detection.confirm()
+        if (s.type === 'receive') s.start().catch(() => {})
+      },
+      (buf) => setTimeout(() => engine.feedZmodem(sessionId, buf), 0)
+    )
+    const events = attachCollecting(sessionId, (id, data) =>
+      setTimeout(() => safeConsume(remote.sentry, data), 0)
+    )
+    // Remote rz initiates: it detects "receive" from ZRQINIT, confirms and
+    // start()s, which sends ZRINIT toward our engine.
+    remote.sentry.consume(Buffer.from(zmodem.Header.build('ZRQINIT').to_hex()))
+
+    const offer = await waitForEvent(events, 'zmodem:offer')
+    if (!offer) fail('detach(cancel): no ZMODEM_OFFER received')
+    if (offer.mode !== 'send') fail(`detach(cancel): expected send offer, got ${offer.mode}`)
+
+    engine.respondZmodem({ id: offer.id, cancelled: false, paths: [] })
+    const done = doneOf(events)
+    if (done.length !== 1) fail(`detach(cancel): expected 1 ZMODEM_DONE, got ${done.length}`)
+    if (done[0].ok) fail('detach(cancel): a cancelled transfer was reported as success')
+    if (engine.isZmodemActive(sessionId)) fail('detach(cancel): engine still active')
+    const prog = progressOf(events)
+    if (prog[prog.length - 1].state !== 'cancelled') {
+      fail(`detach(cancel): last progress state ${prog[prog.length - 1].state}, expected cancelled`)
+    }
+    console.log('[detach] confirmed session cancelled -> done(ok=false), no fake success')
+  }
+}
+
 // ---- run ----------------------------------------------------------------------
 const timer = setTimeout(() => fail('zmodem-e2e timed out'), 30000)
 try {
   await scenarioDownload()
   await scenarioUpload()
+  await scenarioThrottle()
+  await scenarioWriteError()
+  await scenarioDetach()
 } finally {
   clearTimeout(timer)
   cleanupTempDirs()