Address CodeRabbit review: .act vs .aco by content, strict corpus walks (#5407, #4496)

- swatches.import with bytes only (no path or format): an .act whose first
  colour is 0, 1, x or 0, 2, x still went to the .aco reader, which ignores
  trailing bytes and returned a handful of junk swatches. At the two .act
  lengths (768/772) an .aco header now counts only when its sections span
  the whole file (new `aco::sections_len`); a Photoshop .aco of exactly
  768 bytes is still an .aco. Test: act_that_starts_like_an_aco_is_still_an_act.
- The .atn corpus tests (psd and engine) skipped unreadable folders and
  entries silently; they now fail with the path.
- act::write doc: the trailer is written when `count` or `transparent` is given.

Co-authored-by: Abdul karim mia <97598943+abdul-karim-mia@users.noreply.github.com>
Co-authored-by: Fabio Lissi <fabio.lissi@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/swatch_cmds.rs

@@ -629,15 +629,16 @@ pub fn write_file(groups: &[Group<Swatch>], format: Format) -> std::result::Resu
 fn import(s: &mut Session, p: &Value) -> Result<Value> {
     const CMD: &str = "swatches.import";
     let (bytes, stem) = crate::preset_import_cmds::file_bytes(p, CMD, ".aco, .act or .ase file")?;
+    // `.act` has no magic: exactly 768/772 bytes. An `.act` whose first colour is `0, 1, x` or
+    // `0, 2, x` starts like an `.aco`, and the .aco reader ignores trailing bytes, so at those
+    // lengths an .aco header only counts when its sections fill the file exactly.
+    let act_len = bytes.len() == photocraft_psd::act::PALETTE_LEN || bytes.len() == photocraft_psd::act::PALETTE_LEN + photocraft_psd::act::TRAILER_LEN;
     let format = match str_param(p, "format") {
         Some(f) => Format::parse(f).ok_or_else(|| bad(CMD, format!("unknown format `{f}` (aco|act|ase)")))?,
         None if ext_format(str_param(p, "path")) == Some(Format::Act) => Format::Act,
         None if bytes.starts_with(photocraft_psd::ase::SIGNATURE) => Format::Ase,
-        None if matches!(bytes.get(..2), Some([0, 1] | [0, 2])) => Format::Aco,
-        // `.act` has no magic: an exact 768/772 bytes that is neither .ase nor .aco.
-        None if bytes.len() == photocraft_psd::act::PALETTE_LEN || bytes.len() == photocraft_psd::act::PALETTE_LEN + photocraft_psd::act::TRAILER_LEN => {
-            Format::Act
-        }
+        None if matches!(bytes.get(..2), Some([0, 1] | [0, 2])) && (!act_len || photocraft_psd::aco::sections_len(&bytes) == Some(bytes.len())) => Format::Aco,
+        None if act_len => Format::Act,
         None => ext_format(str_param(p, "path")).ok_or_else(|| bad(CMD, "not an .aco, .act or .ase swatch file"))?,
     };
     let mode = str_param(p, "mode").unwrap_or("append");
@@ -852,7 +853,7 @@ pub fn specs() -> Vec<CommandSpec> {
             label: "Import Swatches…",
             menu: &[],
             shortcut: None,
-            params: r##"{"path":".aco, .act or .ase file"?,"data":base64 bytes?,"format":"aco|act|ase"? (detected),"mode":"append|replace"="append","group":str?=file name (the group for ungrouped colours)} → {format,mode,groups,count,warnings}"##,
+            params: r##"{"path":".aco, .act or .ase file"?,"data":base64 bytes?,"format":"aco|act|ase"? (detected; an .act by its .act path or its 768/772-byte length),"mode":"append|replace"="append","group":str?=file name (the group for ungrouped colours)} → {format,mode,groups,count,warnings}"##,
             enabled: always,
             run: import,
             journal: true,

crates/engine/src/swatch_cmds/tests.rs

@@ -179,6 +179,31 @@ fn act_round_trip_loads_a_photoshop_color_table() {
     assert!(r["warnings"].as_array().unwrap().iter().any(|w| w.as_str().is_some_and(|w| w.contains("first 256"))));
 }
 
+#[test]
+fn act_that_starts_like_an_aco_is_still_an_act() {
+    // First colour 0, 1, 0 and second colour 5, …: the first four bytes read as an .aco version 1
+    // header with five swatches, and the .aco reader would take 50 bytes of junk from it.
+    let mut colors = [[0u8; 3]; 256];
+    colors[0] = [0, 1, 0];
+    for (i, c) in colors.iter_mut().enumerate().skip(1) {
+        *c = [(i as u8).wrapping_add(4), 7, 9];
+    }
+    for file in [photocraft_psd::act::write(&colors, None, None).unwrap(), photocraft_psd::act::write(&colors, Some(256), None).unwrap()] {
+        assert_eq!(&file[..4], &[0, 1, 0, 5]);
+        let mut s = Session::new();
+        let r = s.execute("swatches.import", json!({"data": b64_encode(&file), "mode": "replace"})).unwrap();
+        assert_eq!((r["format"].as_str(), r["count"].as_u64()), (Some("act"), Some(256)), "{} bytes", file.len());
+    }
+    // A real .aco of exactly 768 bytes (ten swatches with 25-character names) stays an .aco.
+    let swatches: Vec<AcoSwatch> = (0..10u16).map(|i| AcoSwatch { name: format!("{i:0>25}"), color: AcoColor::Rgb([i * 1000, 0, 65535]) }).collect();
+    let file = photocraft_psd::aco::write(&swatches).unwrap();
+    assert_eq!(file.len(), photocraft_psd::act::PALETTE_LEN);
+    let mut s = Session::new();
+    let r = s.execute("swatches.import", json!({"data": b64_encode(&file), "mode": "replace"})).unwrap();
+    assert_eq!((r["format"].as_str(), r["count"].as_u64()), (Some("aco"), Some(10)));
+    assert_eq!(s.presets.swatches[0].items[9].name, format!("{:0>25}", 9));
+}
+
 #[test]
 fn ase_round_trip_keeps_groups_floats_and_kinds() {
     let src = AseFile {

crates/engine/tests/corpus_atn.rs

@@ -10,9 +10,10 @@ use photocraft_engine::Session;
 use serde_json::json;
 
 fn collect(dir: &Path, out: &mut Vec<PathBuf>) {
-    let Ok(rd) = std::fs::read_dir(dir) else { return };
-    for e in rd.flatten() {
-        let p = e.path();
+    // An unreadable folder fails the test rather than shrinking the corpus.
+    let rd = std::fs::read_dir(dir).unwrap_or_else(|e| panic!("{}: {e}", dir.display()));
+    for e in rd {
+        let p = e.unwrap_or_else(|e| panic!("{}: {e}", dir.display())).path();
         if p.is_dir() {
             collect(&p, out);
         } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("atn")) {

crates/psd/src/aco.rs

@@ -166,6 +166,36 @@ pub fn parse(data: &[u8]) -> Result<AcoFile> {
     }
 }
 
+/// How many bytes the `.aco` sections at the start of `data` span (what [`parse`] reads), or
+/// `None` when they are truncated or the version is unknown. Tells a real `.aco` apart from
+/// bytes that only start like one, such as an `.act` whose first colour is `0, 1, x`.
+pub fn sections_len(data: &[u8]) -> Option<usize> {
+    fn skip_section(r: &mut Reader<'_>, named: bool) -> Result<()> {
+        let count = r.u16()?;
+        for _ in 0..count {
+            r.skip(10)?;
+            if named {
+                let n = u64::from(r.u32()?);
+                r.bytes_u64(n * 2)?;
+            }
+        }
+        Ok(())
+    }
+    let mut r = Reader::new(data);
+    match r.u16().ok()? {
+        1 => {
+            skip_section(&mut r, false).ok()?;
+            if r.remaining() >= 4 && r.peek_rest().get(..2) == Some(&[0, 2]) {
+                r.u16().ok()?;
+                skip_section(&mut r, true).ok()?;
+            }
+        }
+        2 => skip_section(&mut r, true).ok()?,
+        _ => return None,
+    }
+    Some(r.pos())
+}
+
 /// Write an `.aco` file: a version 1 section followed by a version 2 section with the names,
 /// as Photoshop writes them. Fails when there are more than [`MAX_SWATCHES`] swatches.
 pub fn write(swatches: &[AcoSwatch]) -> Result<Vec<u8>> {
@@ -210,6 +240,18 @@ mod tests {
         ]
     }
 
+    #[test]
+    fn sections_len_is_what_parse_reads() {
+        let file = write(&sample()).unwrap();
+        assert_eq!(sections_len(&file), Some(file.len()));
+        let mut padded = file.clone();
+        padded.extend_from_slice(&[0; 6]);
+        assert_eq!(sections_len(&padded), Some(file.len()), "trailing bytes are not part of it");
+        assert_eq!(sections_len(&file[..file.len() - 1]), None, "truncated");
+        assert_eq!(sections_len(&[0, 3, 0, 0]), None, "unknown version");
+        assert_eq!(sections_len(&[0, 1, 0, 0]), Some(4));
+    }
+
     #[test]
     fn round_trips_every_space_at_16_bits() {
         let s = sample();

crates/psd/src/act.rs

@@ -58,7 +58,7 @@ pub fn parse(data: &[u8]) -> Result<ActFile> {
     Ok(ActFile { colors, count, transparent })
 }
 
-/// Write an `.act` file: the 256-color palette plus the trailer when `file` has one.
+/// Write an `.act` file: the 256-color palette plus the trailer when `count` or `transparent` is given.
 /// Short palettes are padded with black; fails on a palette longer than 256 colors.
 pub fn write(colors: &[[u8; 3]], count: Option<u16>, transparent: Option<u16>) -> Result<Vec<u8>> {
     if colors.len() > COLOR_COUNT {

crates/psd/tests/corpus_atn.rs

@@ -8,9 +8,10 @@ use std::path::{Path, PathBuf};
 use photocraft_psd::atn;
 
 fn collect(dir: &Path, out: &mut Vec<PathBuf>) {
-    let Ok(rd) = std::fs::read_dir(dir) else { return };
-    for e in rd.flatten() {
-        let p = e.path();
+    // An unreadable folder fails the test rather than shrinking the corpus.
+    let rd = std::fs::read_dir(dir).unwrap_or_else(|e| panic!("{}: {e}", dir.display()));
+    for e in rd {
+        let p = e.unwrap_or_else(|e| panic!("{}: {e}", dir.display())).path();
         if p.is_dir() {
             collect(&p, out);
         } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("atn")) {