fix(graphics): abort an unfinished kitty upload on delete, sixel or RIS

CHANGELOG.md

@@ -101,6 +101,16 @@ reads that marker.
   also write a character XML 1.0 forbids, a C0 control other than tab, LF and
   CR or U+FFFE/U+FFFF, as U+FFFD, so a window title holding `\x01` or a cell
   holding U+FFFF no longer makes an SVG no parser opens.
+- A chunked kitty upload (`m=1`) is no longer completed by an escape that
+  cannot continue it (#587). A sixel sent mid-upload was built on top of
+  it: one payload, a sixel carrying the kitty `s=`, `c=`/`r=`, `t=` and
+  `i=`, that decoded to a transparent 4x6 instead of the red it painted. A
+  delete (`a=d`) was taken as the upload's last chunk, so `kitty()` counted
+  an image and `deletes()` missed the teardown; any other APC was taken as
+  a chunk too, and after `RIS` the next transmission completed the upload.
+  Only a kitty escape continues an upload now: a delete aborts it, as the
+  protocol requires, and is counted, and a sixel or `RIS` abandons it. An
+  abandoned upload is no image, and its bytes still count in `bytes()`.
 
 ## [0.11.6] - 2026-10-14
 

crates/termlens/src/emu/seq.rs

@@ -853,6 +853,19 @@ impl SeqTracker {
         self.counts
     }
 
+    /// Abort a kitty upload still waiting for its last chunk, keeping the
+    /// wire cost it already ran up.
+    ///
+    /// Only a kitty escape continues an upload. A delete aborts one, in the
+    /// protocol's own words, and neither a sixel nor a hard reset can be
+    /// part of one. All three used to leave the upload in place (#587): the
+    /// sixel was built on top of it and came out carrying the kitty `s=`,
+    /// `c=` and `i=`, the delete was swallowed as its last chunk, and after
+    /// `RIS` the next transmission completed it.
+    fn abandon_upload(&mut self) {
+        self.counts.bytes += self.building.abandon();
+    }
+
     /// True while the application has focus reporting (mode 1004) enabled.
     pub(crate) fn focus_events(&self) -> bool {
         self.focus_events
@@ -929,7 +942,10 @@ impl SeqTracker {
     /// bell count and the link log stay, for the same reason they survive
     /// `RIS`: they are records of what the application emitted, not modes
     /// the terminal holds. An open link span stays open too — a soft reset
-    /// clears no screen, so the text after it is still the span's text.
+    /// clears no screen, so the text after it is still the span's text. A
+    /// kitty upload still in progress survives as well, though `RIS`
+    /// abandons one: kitty's soft reset leaves its images alone, and the
+    /// next chunk there still completes the upload.
     fn soft_reset(&mut self) {
         self.reset_charsets();
         self.saved_charsets = None;
@@ -1359,16 +1375,21 @@ impl SeqTracker {
                 }
             }
         }
-        // An APC opening with `G` is the kitty graphics protocol — or a
-        // continuation of one, which carries `m=` and nothing else.
-        if self.dcs_introducer == b'_'
-            && (self.dcs_head.first() == Some(&b'G') || self.building.in_progress())
-        {
+        // An APC opening with `G` is the kitty graphics protocol — a
+        // continuation chunk too, which is `G` with `m=` and nothing else.
+        // Any other APC is not graphics and leaves an upload where it was,
+        // as kitty does; it used to be taken as the upload's next chunk.
+        if self.dcs_introducer == b'_' && self.dcs_head.first() == Some(&b'G') {
             let control = self.dcs_control_block().to_vec();
             // The control block is `G` followed by the comma-separated keys.
             let keys = control.strip_prefix(b"G").unwrap_or(&control);
             let is_query = keys.split(|&b| b == b',').any(|pair| pair == b"a=q");
             if !is_query {
+                // A delete is never an upload's last chunk: it aborts the
+                // upload in progress and is then a delete like any other.
+                if key_is(keys, b"a", b"d") {
+                    self.abandon_upload();
+                }
                 // One image, however many escapes it took. `m=1` says
                 // another follows; the transmission is not an image until
                 // one arrives without it, and counting each escape instead
@@ -1407,6 +1428,9 @@ impl SeqTracker {
         // intermediate is the whole distinction from the two DCS questions
         // below, which reach the same `q` final through `+` and `$`.
         if self.dcs_introducer == b'P' && self.dcs_final == b'q' && self.dcs_intermediate == 0 {
+            // No part of a kitty upload, so it abandons one still in
+            // progress rather than being built on top of it.
+            self.abandon_upload();
             self.building.sixel();
             let at = self.dcs_payload_start();
             self.building.chunk(
@@ -1634,8 +1658,10 @@ impl SeqTracker {
                 // RIS (`ESC c`), the hard reset: the terminal returns to its
                 // power-on state, so the cursor is the terminal's default
                 // again, all four character sets are ASCII with G0 invoked,
-                // no single shift is pending, and any open hyperlink span
-                // cannot survive.
+                // no single shift is pending, and neither an open hyperlink
+                // span nor a kitty upload still waiting for its last chunk
+                // can survive — kitty's own reset drops the image being
+                // loaded, so the chunks still to come fail there.
                 //
                 // Reporting the last `DECSCUSR` after a reset would claim a
                 // fact the terminal does not hold — and it would do it in the
@@ -1658,6 +1684,7 @@ impl SeqTracker {
                     self.insert_mode = false;
                     self.tabs.reset();
                     self.close_link();
+                    self.abandon_upload();
                     // The grid rebuild is vt100's; the event tells the
                     // emulator to capture what scrolled before it, since
                     // that history is what the rebuild discards (#391).
@@ -1814,7 +1841,7 @@ impl SeqTracker {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::graphics::GraphicsFormat;
+    use crate::graphics::{GraphicsAction, GraphicsFormat, GraphicsProtocol};
 
     /// The width these tests construct a tracker at. Only the tab-stop set
     /// is sized, and only the tab tests care what the size is; the rest is
@@ -1955,6 +1982,125 @@ mod tests {
         assert!(counts.bytes > 22);
     }
 
+    /// The first chunk of a kitty upload whose last chunk never comes, with
+    /// every fact a control block can state, so one leaking into whatever
+    /// follows is visible. #587's first sequence.
+    const KITTY_UNFINISHED: &[u8] = b"\x1b_Ga=T,f=32,s=1,v=1,i=42,c=3,r=2,o=z,t=f,m=1;AAAA\x1b\\";
+    /// A 4x6 sixel, red throughout.
+    const RED_SIXEL: &[u8] = b"\x1bPq\"1;1;4;6#0;2;100;0;0~~~~\x1b\\";
+    /// The first chunk of #587's second sequence.
+    const UPLOAD_OF_5: &[u8] = b"\x1b_Ga=T,f=32,s=2,v=1,i=5,m=1;AAAA\x1b\\";
+
+    /// Wire bytes the tracker counts for whole escapes: everything between
+    /// the two-byte introducer and the two-byte `ST`.
+    fn wire_cost(escapes: &[&[u8]]) -> u64 {
+        escapes.iter().map(|escape| escape.len() as u64 - 4).sum()
+    }
+
+    /// A sixel is no part of a kitty upload, so one arriving mid-upload
+    /// abandons it (#587). It was built on top of it instead: one payload,
+    /// described as a sixel carrying the kitty `s=`, `c=`/`r=`, `t=`, `i=`
+    /// and `o=`, its data the kitty chunk and the sixel run together.
+    #[test]
+    fn a_sixel_abandons_an_unfinished_kitty_upload() {
+        let wire = [KITTY_UNFINISHED, RED_SIXEL].concat();
+        let seen = payloads(&wire);
+        assert_eq!(seen.len(), 1, "{seen:?}");
+        let sixel = &seen[0];
+        assert_eq!(sixel.protocol(), GraphicsProtocol::Sixel);
+        assert_eq!(sixel.size(), Some((4, 6)), "its own raster: {sixel:?}");
+        assert_eq!(sixel.cells(), None, "no kitty fact: {sixel:?}");
+        assert_eq!(sixel.transmission(), None, "{sixel:?}");
+        assert_eq!(sixel.id(), None, "{sixel:?}");
+        assert!(!sixel.compressed(), "{sixel:?}");
+        assert_eq!(sixel.chunks(), 1);
+        assert_eq!(sixel.data(), Some(&b"\"1;1;4;6#0;2;100;0;0~~~~"[..]));
+
+        let counts = fed(&wire).graphics();
+        assert_eq!((counts.kitty, counts.sixel), (0, 1), "{counts:?}");
+        // Abandoned is not unsent: the upload's chunk crossed the wire, and
+        // `bytes` counts traffic rather than pictures.
+        assert_eq!(counts.bytes, wire_cost(&[KITTY_UNFINISHED, RED_SIXEL]));
+    }
+
+    /// The picture the bug was about: built on the kitty chunk, the `AAAA`
+    /// in front pushed the red four columns right of the declared 4x6 and
+    /// the frame decoded fully transparent.
+    #[cfg(feature = "decode")]
+    #[test]
+    fn a_sixel_after_an_abandoned_upload_decodes_to_the_red_it_painted() {
+        let seen = payloads(&[KITTY_UNFINISHED, RED_SIXEL].concat());
+        let bitmap = seen[0].decode().expect("decodes");
+        assert_eq!((bitmap.width(), bitmap.height()), (4, 6));
+        assert_eq!(bitmap.colours(), vec![([255, 0, 0, 255], 24)]);
+    }
+
+    /// "If an image is being loaded in chunks and the upload is not complete
+    /// when any delete command is received, the partial upload must be
+    /// aborted" — and the delete still deletes (#587). It was read as the
+    /// upload's last chunk instead: its control block ignored, the upload
+    /// counted as an image that decodes to nothing, and the teardown
+    /// `deletes` exists to assert gone.
+    #[test]
+    fn a_delete_aborts_an_unfinished_upload_and_is_counted() {
+        let delete: &[u8] = b"\x1b_Ga=d,d=i,i=5\x1b\\";
+        let wire = [UPLOAD_OF_5, delete].concat();
+        let counts = fed(&wire).graphics();
+        assert_eq!(counts.kitty, 0, "an aborted upload is no image: {counts:?}");
+        assert_eq!(counts.deletes, 1, "and the delete is counted: {counts:?}");
+        assert_eq!(counts.bytes, wire_cost(&[UPLOAD_OF_5, delete]));
+
+        let seen = payloads(&wire);
+        assert_eq!(seen.len(), 1, "{seen:?}");
+        assert_eq!(seen[0].action(), GraphicsAction::Delete);
+        assert_eq!(seen[0].id(), Some(5));
+        assert_eq!(seen[0].size(), None, "no fact of the upload's: {seen:?}");
+        assert_eq!(seen[0].chunks(), 1);
+    }
+
+    /// A terminal at power-on has no upload half-received, and kitty's own
+    /// `RIS` drops the image being loaded, so the chunks still to come fail.
+    /// The next transmission is then described by its own control block
+    /// rather than completing the abandoned one (#587). `DECSTR` resets
+    /// modes and keeps content, and kitty's keeps the upload too, so the
+    /// next chunk after it completes the upload as if nothing had come
+    /// between.
+    #[test]
+    fn a_hard_reset_abandons_an_unfinished_upload_and_a_soft_reset_does_not() {
+        let fresh: &[u8] = b"\x1b_Ga=T,f=24,s=1,v=1,i=9;QUJD\x1b\\";
+        let seen = payloads(&[UPLOAD_OF_5, b"\x1bc", fresh].concat());
+        assert_eq!(seen.len(), 1, "{seen:?}");
+        assert_eq!(seen[0].id(), Some(9), "its own control block: {seen:?}");
+        assert_eq!(seen[0].format(), GraphicsFormat::Rgb);
+        assert_eq!(seen[0].size(), Some((1, 1)));
+        assert_eq!(seen[0].chunks(), 1);
+        assert_eq!(seen[0].data(), Some(&b"QUJD"[..]));
+
+        let seen = payloads(&[UPLOAD_OF_5, b"\x1b[!p", b"\x1b_Gm=0;BBBB\x1b\\"].concat());
+        assert_eq!(seen.len(), 1, "{seen:?}");
+        assert_eq!(seen[0].id(), Some(5), "the upload survived: {seen:?}");
+        assert_eq!(seen[0].chunks(), 2);
+        assert_eq!(seen[0].data(), Some(&b"AAAABBBB"[..]));
+    }
+
+    /// Only an APC opening with `G` is kitty's, continuation chunks
+    /// included. Any other APC used to be taken as the upload's next chunk —
+    /// and, carrying no `m=1`, as its last. kitty ignores one, and so the
+    /// upload goes on to the chunk that really is its last.
+    #[test]
+    fn an_apc_that_is_not_kitty_leaves_an_upload_alone() {
+        let wire = [
+            UPLOAD_OF_5,
+            b"\x1b_Xnot kitty;BBBB\x1b\\",
+            b"\x1b_Gm=0;CCCC\x1b\\",
+        ]
+        .concat();
+        let seen = payloads(&wire);
+        assert_eq!(seen.len(), 1, "{seen:?}");
+        assert_eq!(seen[0].chunks(), 2);
+        assert_eq!(seen[0].data(), Some(&b"AAAACCCC"[..]));
+    }
+
     /// A payload past the capture bound keeps every count and drops the
     /// data, rather than handing back a prefix that would decode into a
     /// plausible-looking wrong picture.

crates/termlens/src/graphics.rs

@@ -905,7 +905,9 @@ impl GraphicsSeen {
     ///
     /// **Images, not escapes.** A transmission split across the protocol's
     /// 4096-byte chunks is one image, and an action carrying no picture — a
-    /// delete above all — is not one at all.
+    /// delete above all — is not one at all. Nor is an upload abandoned
+    /// before its last chunk: a delete aborts one, as the protocol requires,
+    /// and so does a sixel or a hard reset (`RIS`) arriving in the middle.
     #[must_use]
     pub fn kitty(&self) -> u32 {
         self.counts.kitty
@@ -945,7 +947,8 @@ impl GraphicsSeen {
 
     /// Total payload bytes across both protocols and every action, counted
     /// the same way for each: everything between the introducer and the
-    /// terminator.
+    /// terminator. The chunks of an abandoned upload count too, since they
+    /// crossed the wire.
     #[must_use]
     pub fn bytes(&self) -> u64 {
         self.counts.bytes
@@ -987,8 +990,9 @@ impl GraphicsSeen {
 }
 
 /// A payload under construction, owned by the sequence tracker: kitty splits
-/// one image across escapes, so a payload is not complete until an escape
-/// arrives without `m=1`.
+/// one image across escapes, so a payload is not complete until a kitty
+/// escape arrives without `m=1` — and it is [abandoned](Self::abandon)
+/// instead if a delete, a sixel or a hard reset arrives first.
 #[derive(Debug)]
 pub(crate) struct GraphicsBuilder {
     protocol: Option<GraphicsProtocol>,
@@ -1027,12 +1031,6 @@ impl Default for GraphicsBuilder {
 }
 
 impl GraphicsBuilder {
-    /// True once a kitty escape has opened a transmission that later
-    /// escapes continue.
-    pub(crate) fn in_progress(&self) -> bool {
-        self.protocol.is_some()
-    }
-
     /// Take on the facts of a kitty control block. Only the first escape of
     /// a chunked transmission carries one; the continuations carry `m=` and
     /// nothing else, so nothing here overwrites what is already known.
@@ -1074,7 +1072,9 @@ impl GraphicsBuilder {
     }
 
     /// Take on the facts of a sixel header: the raster attributes are inside
-    /// the data, so only the protocol is settled here.
+    /// the data, so only the protocol is settled here. Nothing else is
+    /// reset, so the builder must hold no kitty upload — the tracker
+    /// abandons one first.
     pub(crate) fn sixel(&mut self) {
         self.protocol = Some(GraphicsProtocol::Sixel);
         self.action = GraphicsAction::TransmitAndPlace;
@@ -1129,6 +1129,17 @@ impl GraphicsBuilder {
         *self = Self::default();
         Some(payload)
     }
+
+    /// Drop a transmission that will never complete, handing back the wire
+    /// bytes its chunks already cost. Nothing of it becomes a payload: the
+    /// protocol aborts the upload, so there is no image to report. The bytes
+    /// are another matter — they crossed the wire, and
+    /// [`GraphicsSeen::bytes`] counts traffic, not pictures.
+    pub(crate) fn abandon(&mut self) -> u64 {
+        let spent = self.bytes;
+        *self = Self::default();
+        spent
+    }
 }
 
 /// The `"Pan;Pad;Ph;Pv` raster attributes at the head of a sixel stream, if

docs/DESIGN.md

@@ -731,15 +731,19 @@ Rules:
    Those counters count **images**, which is not the same as counting
    escapes, and the difference is not a detail. The kitty protocol caps a
    payload at 4096 bytes and continues with `m=1`, so every image of
-   consequence arrives in several escapes and the continuations carry no
-   control block at all; and `a=d` deletes an image rather than sending one.
-   Counted per escape, a 4.9 KB chart read as two pictures and a teardown
-   read as a third — so "how many images did this frame send?", the question
-   that distinguishes a diffing painter from one that re-sends the year,
-   could not be asked. `GraphicsBuilder` joins the chunks and classifies the
-   action; `deletes` is counted separately, because an application that
-   takes down what it drew and one that draws twice as much are opposite
-   behaviours.
+   consequence arrives in several escapes and the continuations carry `m=`
+   and none of the image's facts; and `a=d` deletes an image rather than
+   sending one. Counted per escape, a 4.9 KB chart read as two pictures and
+   a teardown read as a third — so "how many images did this frame send?",
+   the question that distinguishes a diffing painter from one that re-sends
+   the year, could not be asked. `GraphicsBuilder` joins the chunks and
+   classifies the action; `deletes` is counted separately, because an
+   application that takes down what it drew and one that draws twice as
+   much are opposite behaviours. Only a kitty escape continues an upload. A
+   delete arriving mid-upload aborts it, as the protocol requires, and so do
+   a sixel and `RIS`: the upload is then no image, though its bytes still
+   count, and the escape that interrupted it stands on its own. `DECSTR`
+   leaves an upload alone, as kitty's does (#587).
 
    The payloads themselves land on the same slot, as `GraphicsSeen::payloads`.
    This is the position `OSC 52` already takes and for the same reason: a