Merge pull request #31 from logos-co/feat/manifest040

feat: manifest 0.4.0 — variant-independent icon in assets/

docs/spec.md

@@ -34,6 +34,8 @@ An LGX package (`.lgx` file) is a gzip-compressed tar archive with the following
 package.lgx (tar.gz)
 ├── manifest.json          # Required - package metadata
 ├── manifest.sig           # Optional - Ed25519 signature with DID identity
+├── assets/                # Optional - variant-independent package assets
+│   └── icon.png           #   Package icon: PNG, exactly 256x256
 ├── variants/              # Required - contains variant directories
 │   ├── <variant-1>/       # Variant directory (lowercase name)
 │   │   └── ...            # Variant contents
@@ -46,25 +48,25 @@ package.lgx (tar.gz)
 ```
 
 **Root Entry Constraints:**
-- Only `manifest.json`, `manifest.cose`, `variants/`, `docs/`, and `licenses/` are permitted at root
+- Only `manifest.json`, `manifest.sig`, `assets/`, `variants/`, `docs/`, and `licenses/` are permitted at root
 - Any other root entries cause validation failure
 - Files directly under `variants/` are forbidden (only directories allowed)
 - This strict structure keeps packages easy to validate and reduces ambiguity
 
 ### Manifest Schema
 
-The current manifest schema is `0.3.0`. It is a UTF-8 encoded JSON file with the following required fields:
+The current manifest schema is `0.4.0`. It is a UTF-8 encoded JSON file with the following required fields:
 
 ```json
 {
-  "manifestVersion": "0.3.0",
+  "manifestVersion": "0.4.0",
   "name": "package-name",
   "version": "1.2.3",
   "description": "Package description",
   "author": "Author Name",
   "type": "library",
   "category": "crypto",
-  "icon": "icon.png",
+  "icon": "assets/icon.png",
   "dependencies": [
     "simple-dep",
     {"name": "ranged-dep", "version": "^1.2.0"},
@@ -88,7 +90,7 @@ The current manifest schema is `0.3.0`. It is a UTF-8 encoded JSON file with the
 | `author` | string | Author/maintainer name | Human metadata |
 | `type` | string | Package type classification | Classification |
 | `category` | string | Package category | Classification |
-| `icon` | string | Relative path to icon file bundled in the package | Display/branding |
+| `icon` | string | Relative path to the icon bundled in the package. At `0.4.0`+ this is `assets/icon.png` — see *Icon contract* below | Display/branding |
 | `dependencies` | array | List of dependency entries — see *Dependency entries* below | Runtime needs |
 | `main` | object | Map of variant name → relative path to entry point (e.g ) `"linux-amd64": "path/to/main.so"` means `linux-amd64/path/to/main.so` | Entry point resolution |
 | `display_name` | string | *Optional.* Human-readable label shown by UI consumers (Package Manager, App Manager) and CLI tools (`lm metadata`, `lgx manifest`). Falls back to `name` when absent. | Display/branding |
@@ -111,6 +113,48 @@ Each element of the `dependencies` array is one of:
 
 Tooling reads both `manifestVersion: "0.2.x"` and `manifestVersion: "0.3.x"`. Packages produced by `lgx create` use `0.3.0`. A 0.2.0 manifest with plain-string dependencies round-trips unchanged through tooling — strings are emitted as strings, object-form entries are emitted as objects. Bumping the major version (1.x.x) is reserved for future breaking changes.
 
+### Icon Contract
+
+At `manifestVersion` `0.4.0`+, `icon` points at a **root-level, variant-independent
+asset**: `assets/icon.png`.
+
+**Requirements**
+
+| Rule | Value |
+|------|-------|
+| Format | PNG |
+| Dimensions | **exactly** 256x256 — not a minimum |
+| Location | `assets/icon.png` (canonical; set by `lgx add --icon`) |
+| Required for | `type == "ui_qml"` |
+| Optional for | every other type, including `core` |
+
+**Why root-level rather than per-variant.** One copy instead of one per
+platform, and a host can read the icon without choosing or unpacking a
+platform build — which is what lets a registry extract it at publish time and
+serve it to clients that have not downloaded the package. `assets/` is inside
+the Merkle tree, so the icon is authenticated by `manifest.sig` for free.
+
+**Why exactly 256x256.** The artifact stays byte-predictable: the icon in
+`assets/` is the author's file unmodified, so the content hash is reproducible
+from source without depending on an image library's resampling behaviour. It
+also keeps the packaging step free of an image-processing dependency —
+dimension validation is a fixed-offset PNG `IHDR` read.
+
+**Why `ui_qml` only.** UI packages render a tile in the App Manager, sidebar
+and launcher. Core modules appear in package lists but have no such surface,
+so requiring artwork from them would be cost without benefit.
+
+**Validation timing.** `lgx create` and `lgx add` are construction steps and do
+not enforce the contract — a package may be assembled in any order. Enforcement
+happens at `lgx verify`, `lgx sign` and `lgpm install`, i.e. wherever a package
+is asserted to be complete.
+
+**Backward compatibility.** Manifests below `0.4.0` are **exempt**. `icon: ""`
+was legal at `0.3.0` (it is what `lgx create` defaulted to), so applying the
+rule unconditionally would make every already-published package fail
+verification and become uninstallable. Tooling reads `0.2.x`, `0.3.x` and
+`0.4.x`; only `0.4.0`+ carries the icon contract.
+
 ### `ui_qml` Contract
 
 For most package types, `main` remains the per-variant entry point and is required.

doctests/lgx-cli.test.yaml

@@ -104,7 +104,7 @@ sections:
       - title: "Read the fresh manifest"
         text: |
           `lgx manifest` prints the embedded `manifest.json` in human-readable form. A
-          freshly created package starts at version `0.0.1`, manifest schema `0.3.0`, with
+          freshly created package starts at version `0.0.1`, manifest schema `0.4.0`, with
           no type, no variants, and no signature yet.
         run: "./lgx/bin/lgx manifest greeter.lgx"
         code_block: "lgx manifest greeter.lgx"
@@ -112,7 +112,7 @@ sections:
           - "greeter"
           - "0.0.1"
           - "Manifest ver.:"
-          - "0.3.0"
+          - "0.4.0"
           - "Variants:       (none)"
           - "Signed:         no"
 

doctests/outputs/lgx-cli.md

@@ -57,7 +57,7 @@ link the result as `./lgx`, so the binary lands at `./lgx/bin/lgx`.
 
 ```bash
 # From inside the clone this is simply: nix build '.#lgx' -o lgx
-nix build 'github:logos-co/logos-package/41cae9e4546899eb11912372588a6707ca43efe4#lgx' -o lgx
+nix build 'github:logos-co/logos-package/43378dab2ffc569a28f144d3f62a0f089c3580df#lgx' -o lgx
 ```
 
 The `-o lgx` flag names the result symlink, so the executable is at `./lgx/bin/lgx`.
@@ -108,7 +108,7 @@ sensible defaults plus an empty `variants/` directory — to `<name>.lgx`.
 ### 3.2 Read the fresh manifest
 
 `lgx manifest` prints the embedded `manifest.json` in human-readable form. A
-freshly created package starts at version `0.0.1`, manifest schema `0.3.0`, with
+freshly created package starts at version `0.0.1`, manifest schema `0.4.0`, with
 no type, no variants, and no signature yet.
 
 ```bash

doctests/outputs/lgx-signing.md

@@ -55,7 +55,7 @@ detail; here they are just setup.)
 
 ```bash
 # From inside the clone this is simply: nix build '.#lgx' -o lgx
-nix build 'github:logos-co/logos-package/41cae9e4546899eb11912372588a6707ca43efe4#lgx' -o lgx
+nix build 'github:logos-co/logos-package/43378dab2ffc569a28f144d3f62a0f089c3580df#lgx' -o lgx
 ```
 
 ### 1.2 Create and populate greeter.lgx

src/commands/add_command.cpp

@@ -3,6 +3,9 @@
 #include "core/path_normalizer.h"
 
 #include <filesystem>
+#include <fstream>
+#include <iterator>
+#include <vector>
 
 namespace lgx {
 
@@ -34,6 +37,7 @@ int AddCommand::execute(const std::vector<std::string>& args) {
     
     std::string mainPath = getOption(opts, "main", "m");
     std::string viewPath = getOption(opts, "view");
+    std::string iconPath = getOption(opts, "icon");
     bool autoYes = hasFlag(opts, "yes", "y");
     
     // Check if package exists
@@ -67,7 +71,32 @@ int AddCommand::execute(const std::vector<std::string>& args) {
         }
         pkg.getManifest().view = viewPath;
     }
-    
+
+    // Apply --icon: stage the file at the canonical assets/ path and point the
+    // manifest at it. Variant-independent by design — one icon serves every
+    // platform build, so this is set on the package rather than per variant.
+    // Validation of the bytes (PNG, exactly 256x256) happens in
+    // Package::validateIconAsset() when the package is verified or signed.
+    if (!iconPath.empty()) {
+        if (!std::filesystem::exists(iconPath)) {
+            printError("Icon not found: " + iconPath);
+            return 1;
+        }
+        std::ifstream iconFile(iconPath, std::ios::binary);
+        if (!iconFile) {
+            printError("Cannot read icon: " + iconPath);
+            return 1;
+        }
+        std::vector<uint8_t> iconData(
+            (std::istreambuf_iterator<char>(iconFile)),
+            std::istreambuf_iterator<char>());
+        auto iconResult = pkg.setIcon(iconData);
+        if (!iconResult.success) {
+            printError("Failed to set icon: " + iconResult.error);
+            return 1;
+        }
+    }
+
     // Check if variant exists (replacement warning)
     bool variantExists = pkg.hasVariant(variantLc);
     

src/commands/add_command.h

@@ -32,6 +32,10 @@ public:
                "  --view <relpath>       QML entry point relative to variant root\n"
                "                         (required for `ui_qml` packages; sets the\n"
                "                          manifest-level `view` field)\n"
+               "  --icon <path>          Package icon: PNG, exactly 256x256. Stored\n"
+               "                         once at assets/icon.png and shared by every\n"
+               "                         variant. Required for `ui_qml` packages at\n"
+               "                         manifest version 0.4.0+\n"
                "  --yes, -y              Skip confirmation prompts\n"
                "\n"
                "Examples:\n"

src/core/manifest.cpp

@@ -412,12 +412,34 @@ bool Manifest::isVersionSupported(const std::string& version) {
     std::string major = version.substr(0, dotPos);
 
     // Currently only major version 0 is supported. Within 0.x we accept
-    // both 0.2.* (legacy plain-string dependencies) and 0.3.* (richer
-    // dependencies with optional version range + signer DID); see the
-    // Dependency parsing in fromJson() for the compatibility shim.
+    // 0.2.* (legacy plain-string dependencies), 0.3.* (richer dependencies
+    // with optional version range + signer DID) and 0.4.* (root-level
+    // assets/ slot); see the Dependency parsing in fromJson() for the
+    // compatibility shim and requiresIconContract() for the icon gate.
     return major == "0";
 }
 
+bool Manifest::requiresIconContract(const std::string& version) {
+    // Parse "<major>.<minor>." — anything below 0.4 predates assets/.
+    const size_t firstDot = version.find('.');
+    if (firstDot == std::string::npos) return false;
+    const size_t secondDot = version.find('.', firstDot + 1);
+
+    const std::string majorStr = version.substr(0, firstDot);
+    const std::string minorStr = secondDot == std::string::npos
+        ? version.substr(firstDot + 1)
+        : version.substr(firstDot + 1, secondDot - firstDot - 1);
+
+    try {
+        const int major = std::stoi(majorStr);
+        const int minor = std::stoi(minorStr);
+        if (major > 0) return true;          // 1.x and beyond
+        return major == 0 && minor >= 4;     // 0.4.0+
+    } catch (const std::exception&) {
+        return false;                        // unparseable → treat as legacy
+    }
+}
+
 Manifest::ValidationResult Manifest::compareMetadata(const Manifest& other) const {
     ValidationResult result = ValidationResult::ok();
 

src/core/manifest.h

@@ -56,10 +56,25 @@ struct Dependency {
  */
 class Manifest {
 public:
-    // Current manifest version. Bumped to 0.3.0 to accommodate richer
-    // dependency entries (semver ranges + optional signer DID). 0.2.x
-    // manifests are still readable for backward compatibility.
-    static constexpr const char* CURRENT_VERSION = "0.3.0";
+    // Current manifest version. Bumped to 0.4.0 for the root-level assets/
+    // slot: `icon` now points at a variant-independent asset (assets/icon.png)
+    // that is exactly 256x256 PNG, so hosts can display it without unpacking
+    // a platform build. 0.2.x and 0.3.x manifests are still readable.
+    static constexpr const char* CURRENT_VERSION = "0.4.0";
+
+    // Canonical in-package icon location for 0.4.0+. The author's
+    // metadata.json path stays free-form; the bundler normalises to this.
+    static constexpr const char* ICON_PATH = "assets/icon.png";
+
+    // Required icon dimensions, enforced for 0.4.0+ packages whose type
+    // requires an icon. Exact, not a minimum — see plan.md §3.4.
+    static constexpr int ICON_SIZE_PX = 256;
+
+    // True when `version` is 0.4.0 or newer, i.e. the icon contract applies.
+    // 0.2.x/0.3.x packages predate it and are exempt (plan.md §3.7): `icon`
+    // was legal as "" back then, so applying the rule unconditionally would
+    // make every already-published package uninstallable.
+    static bool requiresIconContract(const std::string& version);
     
     /**
      * Validation result for manifest.

src/core/package.cpp

@@ -10,10 +10,121 @@ namespace lgx {
 
 thread_local std::string Package::lastError_;
 
+namespace {
+
+// PNG header layout is fixed-offset, so dimensions come out of the first 26
+// bytes with no image library: 8-byte signature, 4-byte IHDR length, 4-byte
+// "IHDR" tag, then width and height as big-endian uint32.
+//
+// This validates the *declared* dimensions only. It is deliberately not a
+// defence against a malicious payload — a PNG can claim 256x256 and carry an
+// enormous IDAT. Byte-size ceilings and decoder allocation limits belong at
+// the fetch/decode boundary (plan.md §3.2.2), not here.
+struct PngHeader {
+    bool valid = false;
+    uint32_t width = 0;
+    uint32_t height = 0;
+};
+
+PngHeader readPngHeader(const std::vector<uint8_t>& data) {
+    static const uint8_t kSignature[8] =
+        {0x89, 'P', 'N', 'G', '\r', '\n', 0x1A, '\n'};
+
+    PngHeader h;
+    if (data.size() < 26) return h;
+    if (!std::equal(std::begin(kSignature), std::end(kSignature), data.begin()))
+        return h;
+    if (data[12] != 'I' || data[13] != 'H' ||
+        data[14] != 'D' || data[15] != 'R') return h;
+
+    auto be32 = [&data](size_t off) {
+        return (static_cast<uint32_t>(data[off])     << 24)
+             | (static_cast<uint32_t>(data[off + 1]) << 16)
+             | (static_cast<uint32_t>(data[off + 2]) << 8)
+             |  static_cast<uint32_t>(data[off + 3]);
+    };
+
+    h.width  = be32(16);
+    h.height = be32(20);
+    h.valid  = true;
+    return h;
+}
+
+} // namespace
+
+void Package::validateIconAsset(VerifyResult& result) const {
+    // Version gate. 0.2.x/0.3.x packages predate the assets/ slot and were
+    // free to carry `icon: ""`; enforcing the contract on them would render
+    // the entire already-published catalog uninstallable. See plan.md §3.7.
+    if (!Manifest::requiresIconContract(manifest_.manifestVersion)) return;
+
+    // Icons are required for UI packages, which are the ones that render a
+    // tile. Core modules appear in package lists but have no launcher or
+    // sidebar presence, so an icon stays optional for them.
+    const bool iconRequired = (manifest_.type == "ui_qml");
+
+    if (manifest_.icon.empty()) {
+        if (iconRequired) {
+            result.valid = false;
+            result.errors.push_back(
+                std::string("Missing 'icon': ") + Manifest::ICON_PATH +
+                " is required for type '" + manifest_.type + "' packages");
+        }
+        return;
+    }
+
+    // The canonical location is part of the contract, not a convention: a
+    // host resolves <installDir>/assets/icon.png without consulting the
+    // manifest, and the release tool globs for it. Allowing `icon` to point
+    // anywhere would make both unpredictable.
+    if (manifest_.icon != Manifest::ICON_PATH) {
+        result.valid = false;
+        result.errors.push_back(
+            std::string("Manifest 'icon' must be '") + Manifest::ICON_PATH +
+            "' at manifestVersion 0.4.0+, got '" + manifest_.icon + "'");
+        return;
+    }
+
+    const TarEntry* iconEntry = nullptr;
+    for (const auto& entry : entries_) {
+        if (entry.path == manifest_.icon && !entry.isDirectory) {
+            iconEntry = &entry;
+            break;
+        }
+    }
+
+    if (!iconEntry) {
+        result.valid = false;
+        result.errors.push_back(
+            "Manifest 'icon' points at '" + manifest_.icon +
+            "' which is not present in the package");
+        return;
+    }
+
+    const PngHeader header = readPngHeader(iconEntry->data);
+    if (!header.valid) {
+        result.valid = false;
+        result.errors.push_back(
+            "Icon '" + manifest_.icon + "' does not match the Logos icon "
+            "standard: expected PNG, exactly 256x256; actual: not a PNG");
+        return;
+    }
+
+    const uint32_t want = static_cast<uint32_t>(Manifest::ICON_SIZE_PX);
+    if (header.width != want || header.height != want) {
+        result.valid = false;
+        result.errors.push_back(
+            "Icon '" + manifest_.icon + "' does not match the Logos icon "
+            "standard: expected PNG, exactly 256x256; actual: PNG, " +
+            std::to_string(header.width) + "x" + std::to_string(header.height));
+    }
+}
+
 const std::set<std::string> Package::ALLOWED_ROOT_ENTRIES = {
     "manifest.json",
     "manifest.sig",
     "variants",
+    "assets",
     "docs",
     "licenses"
 };
@@ -246,6 +357,8 @@ Package::VerifyResult Package::validatePackage() const {
         result.errors.push_back("Missing variants/ directory");
     }
 
+    validateIconAsset(result);
+
     // Validate completeness (variants <-> main mapping)
     auto completenessResult = manifest_.validateCompleteness(foundVariants);
     if (!completenessResult.valid) {
@@ -656,8 +769,20 @@ Package::Result Package::extractVariant(
 
     std::string prefix = "variants/" + variantLc + "/";
 
+    // Root-level `assets/` is variant-independent and must land in the SAME
+    // output directory as the variant contents, because the manifest's `icon`
+    // is documented as relative to the installed package root. Extracting
+    // only `variants/<v>/` left `assets/icon.png` on the floor, so every
+    // installed 0.4.0 package resolved its icon to a missing file and fell
+    // back to the monogram — a regression against 0.3.x, where the icon lived
+    // inside the variant and therefore did extract.
+    const std::string assetsPrefix = "assets/";
+
     for (const auto& entry : entries_) {
-        if (entry.path.substr(0, prefix.length()) != prefix) {
+        const bool inVariant = entry.path.compare(0, prefix.length(), prefix) == 0;
+        const bool inAssets =
+            entry.path.compare(0, assetsPrefix.length(), assetsPrefix) == 0;
+        if (!inVariant && !inAssets) {
             continue;
         }
 
@@ -673,7 +798,10 @@ Package::Result Package::extractVariant(
                                 entry.path + "': " + pathValidation.error);
         }
 
-        std::string relativePath = entry.path.substr(prefix.length());
+        // Variant entries are rebased to the variant root; asset entries keep
+        // their `assets/...` path so the installed layout matches the manifest.
+        std::string relativePath =
+            inVariant ? entry.path.substr(prefix.length()) : entry.path;
         if (relativePath.empty()) {
             continue;
         }
@@ -873,6 +1001,39 @@ void Package::clearSignature() {
     manifestSig_ = std::nullopt;
 }
 
+Package::Result Package::setIcon(const std::vector<uint8_t>& pngData) {
+    if (pngData.empty()) {
+        return Result::fail("Icon data is empty");
+    }
+
+    const std::string iconPath = Manifest::ICON_PATH;
+
+    // Replace any existing entry at the canonical path.
+    entries_.erase(
+        std::remove_if(entries_.begin(), entries_.end(),
+                       [&iconPath](const TarEntry& e) {
+                           return e.path == iconPath;
+                       }),
+        entries_.end());
+
+    TarEntry iconEntry;
+    iconEntry.path = iconPath;
+    iconEntry.data = pngData;
+    iconEntry.isDirectory = false;
+    entries_.push_back(std::move(iconEntry));
+
+    manifest_.icon = iconPath;
+
+    // Invalidate signature and recompute hashes (content changed)
+    clearSignature();
+    auto hashResult = recomputeHashes();
+    if (!hashResult.success) {
+        return hashResult;
+    }
+
+    return Result::ok();
+}
+
 Package::Result Package::recomputeHashes() {
     if (!crypto::init()) {
         return Result::fail("Failed to initialize crypto library — cannot compute content hashes");

src/core/package.h

@@ -83,6 +83,22 @@ public:
      */
     static VerifyResult verify(const std::filesystem::path& lgxPath);
     
+    /**
+     * Set (or replace) the package icon.
+     *
+     * Writes the bytes to the canonical assets/icon.png entry and points the
+     * manifest's `icon` field at it. The icon is variant-independent: one
+     * copy serves every platform build, and a host can read it without
+     * choosing or unpacking a variant.
+     *
+     * Byte validation (PNG, exactly 256x256) is deliberately NOT done here —
+     * it happens in validateIconAsset() at verify/sign time, so a package can
+     * be assembled in any order.
+     *
+     * Invalidates any existing signature and recomputes content hashes.
+     */
+    Result setIcon(const std::vector<uint8_t>& pngData);
+
     /**
      * Add files to a variant.
      * If the variant exists, it is completely replaced.
@@ -232,6 +248,16 @@ private:
     
     static thread_local std::string lastError_;
     
+    /**
+     * Validate the packaged icon asset against the Logos icon standard
+     * (PNG, exactly 256x256, at the manifest's `icon` path).
+     *
+     * No-op for manifests older than 0.4.0 — they predate the assets/ slot
+     * and legitimately carry `icon: ""`, so enforcing would break every
+     * already-published package. See Manifest::requiresIconContract().
+     */
+    void validateIconAsset(VerifyResult& result) const;
+
     /**
      * Rebuild tar entries, ensuring manifest is included.
      */

tests/test_cli.cpp

@@ -3,6 +3,8 @@
 #include <cstdlib>
 #include <filesystem>
 #include <fstream>
+
+#include "test_png.h"
 #include <sstream>
 
 namespace fs = std::filesystem;
@@ -329,9 +331,14 @@ TEST_F(CLITest, AddCommand_UiQmlDirectoryWithoutMain) {
     auto saveResult = pkgOpt->save(pkgPath);
     ASSERT_TRUE(saveResult.success);
 
+    // 0.4.0 requires ui_qml packages to carry a conforming icon.
+    const std::string iconPath =
+        lgx_test::writePng((tempDir / "icon.png").string());
+
     std::string output;
     int exitCode = runLgx(
-        "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string() + " -y",
+        "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string() +
+        " --icon " + iconPath + " -y",
         &output
     );
 
@@ -356,10 +363,14 @@ TEST_F(CLITest, AddCommand_UiQmlDirectoryWithViewFlag) {
     auto saveResult = pkgOpt->save(pkgPath);
     ASSERT_TRUE(saveResult.success);
 
+    // 0.4.0 requires ui_qml packages to carry a conforming icon.
+    const std::string iconPath =
+        lgx_test::writePng((tempDir / "icon.png").string());
+
     std::string output;
     int exitCode = runLgx(
         "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string()
-            + " --view qml/Main.qml -y",
+            + " --view qml/Main.qml --icon " + iconPath + " -y",
         &output
     );
 
@@ -388,9 +399,14 @@ TEST_F(CLITest, AddCommand_UiQmlDirectoryWithoutView) {
     auto saveResult = pkgOpt->save(pkgPath);
     ASSERT_TRUE(saveResult.success);
 
+    // 0.4.0 requires ui_qml packages to carry a conforming icon.
+    const std::string iconPath =
+        lgx_test::writePng((tempDir / "icon.png").string());
+
     std::string output;
     int exitCode = runLgx(
-        "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string() + " -y",
+        "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string() +
+        " --icon " + iconPath + " -y",
         &output
     );
 

tests/test_package.cpp

@@ -8,6 +8,9 @@
 
 #include <filesystem>
 #include <fstream>
+#include <vector>
+
+#include "test_png.h"
 
 using namespace lgx;
 namespace fs = std::filesystem;
@@ -234,6 +237,8 @@ TEST_F(PackageTest, AddVariant_UiQmlDirectory_AllowsMissingMain) {
     ASSERT_TRUE(pkg.has_value());
     pkg->getManifest().type = "ui_qml";
     pkg->getManifest().view = "qml/Main.qml";
+    // 0.4.0 requires ui_qml packages to carry a conforming icon.
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success);
 
     fs::path testDir = tempDir / "dist";
     createTestDirectory(testDir, {
@@ -262,6 +267,8 @@ TEST_F(PackageTest, AddVariant_UiQmlDirectory_ClearsStaleMain) {
     ASSERT_TRUE(pkg.has_value());
     pkg->getManifest().type = "ui_qml";
     pkg->getManifest().view = "qml/Main.qml";
+    // 0.4.0 requires ui_qml packages to carry a conforming icon.
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success);
 
     fs::path firstDir = tempDir / "first";
     createTestDirectory(firstDir, {
@@ -276,6 +283,8 @@ TEST_F(PackageTest, AddVariant_UiQmlDirectory_ClearsStaleMain) {
     ASSERT_TRUE(pkg.has_value());
     pkg->getManifest().type = "ui_qml";
     pkg->getManifest().view = "qml/Main.qml";
+    // 0.4.0 requires ui_qml packages to carry a conforming icon.
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success);
 
     fs::path secondDir = tempDir / "second";
     createTestDirectory(secondDir, {
@@ -1310,3 +1319,279 @@ TEST_F(PackageTest, Verify_SignedPackage_ValidHashes) {
     EXPECT_TRUE(result.valid) << "Errors: " <<
         (result.errors.empty() ? "none" : result.errors[0]);
 }
+
+// =============================================================================
+// Icon Contract (manifest 0.4.0+) — plan.md §3.4, §3.7
+// =============================================================================
+
+TEST_F(PackageTest, Icon_ConformingIconValidates) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    pkg->getManifest().type = "ui_qml";
+    pkg->getManifest().view = "qml/Main.qml";
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success);
+
+    fs::path testDir = tempDir / "dist";
+    createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}});
+    ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success);
+    ASSERT_TRUE(pkg->save(pkgPath).success);
+
+    auto result = Package::verify(pkgPath);
+    EXPECT_TRUE(result.valid) << "Errors: " <<
+        (result.errors.empty() ? "none" : result.errors[0]);
+}
+
+// The icon lands at the canonical root path, NOT inside a variant — that is
+// what makes it readable without unpacking a platform build.
+TEST_F(PackageTest, Icon_StoredAtCanonicalRootPath) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success);
+    ASSERT_TRUE(pkg->save(pkgPath).success);
+
+    pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    EXPECT_EQ(pkg->getManifest().icon, std::string("assets/icon.png"));
+
+    bool found = false;
+    for (const auto& e : pkg->getEntries())
+        if (e.path == "assets/icon.png" && !e.isDirectory) found = true;
+    EXPECT_TRUE(found) << "icon must live at assets/icon.png";
+}
+
+TEST_F(PackageTest, Icon_WrongDimensionsRejected) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    pkg->getManifest().type = "ui_qml";
+    pkg->getManifest().view = "qml/Main.qml";
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng(512, 512)).success);
+
+    fs::path testDir = tempDir / "dist";
+    createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}});
+    ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success);
+    pkg->save(pkgPath);
+
+    auto result = Package::verify(pkgPath);
+    EXPECT_FALSE(result.valid);
+    ASSERT_FALSE(result.errors.empty());
+    // The message must name the standard and both sizes, not just assert.
+    EXPECT_NE(result.errors[0].find("256x256"), std::string::npos);
+    EXPECT_NE(result.errors[0].find("512x512"), std::string::npos);
+}
+
+TEST_F(PackageTest, Icon_NonPngRejected) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    pkg->getManifest().type = "ui_qml";
+    pkg->getManifest().view = "qml/Main.qml";
+    std::vector<uint8_t> notPng(64, 0x41);  // "AAAA..."
+    ASSERT_TRUE(pkg->setIcon(notPng).success);
+
+    fs::path testDir = tempDir / "dist";
+    createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}});
+    ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success);
+    pkg->save(pkgPath);
+
+    auto result = Package::verify(pkgPath);
+    EXPECT_FALSE(result.valid);
+    ASSERT_FALSE(result.errors.empty());
+    EXPECT_NE(result.errors[0].find("not a PNG"), std::string::npos);
+}
+
+TEST_F(PackageTest, Icon_MissingOnUiQmlRejected) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    pkg->getManifest().type = "ui_qml";
+    pkg->getManifest().view = "qml/Main.qml";
+
+    fs::path testDir = tempDir / "dist";
+    createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}});
+    ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success);
+    pkg->save(pkgPath);
+
+    auto result = Package::verify(pkgPath);
+    EXPECT_FALSE(result.valid);
+}
+
+// Core modules appear in package lists but render no tile, so an icon stays
+// optional for them. Requiring one would turn a 12-module migration into a
+// 33-module one for no visual benefit.
+TEST_F(PackageTest, Icon_OptionalForCoreType) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    pkg->getManifest().type = "core";
+
+    fs::path testFile = tempDir / "mod.so";
+    createTestFile(testFile, "binary");
+    ASSERT_TRUE(pkg->addVariant("linux-amd64", testFile).success);
+    ASSERT_TRUE(pkg->save(pkgPath).success);
+
+    auto result = Package::verify(pkgPath);
+    EXPECT_TRUE(result.valid) << "Errors: " <<
+        (result.errors.empty() ? "none" : result.errors[0]);
+}
+
+// THE backward-compatibility regression. A 0.3.0 ui_qml package with icon:""
+// was legal and is published in the wild. Enforcing the 0.4.0 contract on it
+// would make the entire existing catalog fail verification and become
+// uninstallable. See plan.md §3.7.
+TEST_F(PackageTest, Icon_LegacyManifestExemptFromContract) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    pkg->getManifest().manifestVersion = "0.3.0";
+    pkg->getManifest().type = "ui_qml";
+    pkg->getManifest().view = "qml/Main.qml";
+    pkg->getManifest().icon = "";
+
+    fs::path testDir = tempDir / "dist";
+    createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}});
+    ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success);
+    ASSERT_TRUE(pkg->save(pkgPath).success);
+
+    auto result = Package::verify(pkgPath);
+    EXPECT_TRUE(result.valid) << "0.3.0 package must remain valid. Errors: " <<
+        (result.errors.empty() ? "none" : result.errors[0]);
+}
+
+// Mutating the icon must change the Merkle root — assets/ is inside the tree,
+// which is what makes the icon signature-covered for free.
+TEST_F(PackageTest, Icon_ParticipatesInMerkleTree) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success);
+    ASSERT_TRUE(pkg->save(pkgPath).success);
+    pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    const std::string rootA = pkg->getManifest().hashes.count("root")
+                              ? pkg->getManifest().hashes.at("root") : "";
+    ASSERT_FALSE(rootA.empty());
+
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng(256, 256)).success);
+    // Same dimensions but different pixel payload would be identical here, so
+    // use a differently-sized image to guarantee different bytes.
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng(128, 128)).success);
+    ASSERT_TRUE(pkg->save(pkgPath).success);
+    pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    const std::string rootB = pkg->getManifest().hashes.count("root")
+                              ? pkg->getManifest().hashes.at("root") : "";
+
+    EXPECT_NE(rootA, rootB) << "assets/ must be covered by the Merkle tree";
+}
+
+// The gap that let the icon regression ship: every existing test checked the
+// archive's CONTENTS, none checked what lands on disk after extraction. The
+// manifest documents `icon` as relative to the installed package root, so the
+// contract is "extract, then resolve manifest.icon" — assert exactly that.
+TEST_F(PackageTest, Icon_ExtractedToInstalledPackageRoot) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    pkg->getManifest().type = "ui_qml";
+    pkg->getManifest().view = "qml/Main.qml";
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success);
+
+    fs::path testDir = tempDir / "dist";
+    createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}});
+    ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success);
+    ASSERT_TRUE(pkg->save(pkgPath).success);
+
+    fs::path out = tempDir / "installed";
+    auto loaded = Package::load(pkgPath);
+    ASSERT_TRUE(loaded.has_value());
+    ASSERT_TRUE(loaded->extractVariant("linux-amd64", out).success);
+
+    // This is the exact join UIPluginManager::pluginIconUrl() performs.
+    const fs::path installDir = out / "linux-amd64";
+    const fs::path resolved   = installDir / loaded->getManifest().icon;
+    EXPECT_TRUE(fs::exists(resolved))
+        << "manifest icon '" << loaded->getManifest().icon
+        << "' must resolve under the installed package root";
+
+    // Variant payload still lands where it always did.
+    EXPECT_TRUE(fs::exists(installDir / "qml" / "Main.qml"));
+
+    // And the extracted icon is byte-identical to what was packaged.
+    std::ifstream f(resolved, std::ios::binary);
+    std::vector<uint8_t> onDisk((std::istreambuf_iterator<char>(f)),
+                                 std::istreambuf_iterator<char>());
+    EXPECT_EQ(onDisk, lgx_test::makePng());
+}
+
+// Assets are variant-independent: extracting a different variant must still
+// produce the icon, or a darwin install would lose what a linux install kept.
+TEST_F(PackageTest, Icon_ExtractedForEveryVariant) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success);
+
+    fs::path a = tempDir / "a"; createTestFile(a / "mod.so", "x");
+    fs::path b = tempDir / "b"; createTestFile(b / "mod.dylib", "y");
+    ASSERT_TRUE(pkg->addVariant("linux-amd64", a, "mod.so").success);
+    ASSERT_TRUE(pkg->addVariant("darwin-arm64", b, "mod.dylib").success);
+    ASSERT_TRUE(pkg->save(pkgPath).success);
+
+    auto loaded = Package::load(pkgPath);
+    ASSERT_TRUE(loaded.has_value());
+    for (const std::string v : {"linux-amd64", "darwin-arm64"}) {
+        fs::path out = tempDir / ("out-" + v);
+        ASSERT_TRUE(loaded->extractVariant(v, out).success) << v;
+        EXPECT_TRUE(fs::exists(out / v / "assets" / "icon.png"))
+            << "variant " << v << " lost the root asset";
+    }
+}
+
+// Review fix: a 0.4.0 package could previously pass with `icon` pointing
+// anywhere, contradicting the canonical-path contract that hosts and the
+// release tool both rely on.
+TEST_F(PackageTest, Icon_NonCanonicalPathRejected) {
+    fs::path pkgPath = tempDir / "test.lgx";
+    Package::create(pkgPath, "testpkg");
+
+    auto pkg = Package::load(pkgPath);
+    ASSERT_TRUE(pkg.has_value());
+    pkg->getManifest().type = "ui_qml";
+    pkg->getManifest().view = "qml/Main.qml";
+    ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success);
+    // Point the manifest somewhere else under assets/ — file still exists.
+    pkg->getManifest().icon = "assets/elsewhere.png";
+
+    fs::path testDir = tempDir / "dist";
+    createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}});
+    ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success);
+    pkg->save(pkgPath);
+
+    auto result = Package::verify(pkgPath);
+    EXPECT_FALSE(result.valid);
+    ASSERT_FALSE(result.errors.empty());
+    EXPECT_NE(result.errors[0].find("assets/icon.png"), std::string::npos);
+}

tests/test_png.h

@@ -0,0 +1,84 @@
+#pragma once
+
+// Shared PNG fixture builder for tests that need a package icon.
+//
+// Package::validateIconAsset() only reads IHDR (width/height at fixed
+// offsets) and never decodes pixels, but we emit a real zlib-compressed
+// IDAT so fixtures are decodable images rather than header-shaped stubs —
+// otherwise a test could pass against a validator that later starts
+// decoding, and we would not find out here.
+
+#include <cstdint>
+#include <fstream>
+#include <string>
+#include <vector>
+
+#include <zlib.h>
+
+namespace lgx_test {
+
+// Default size is the 256x256 the Logos icon standard requires; pass another
+// size to exercise rejection.
+inline std::vector<uint8_t> makePng(uint32_t w = 256, uint32_t h = 256) {
+    auto be32 = [](uint32_t v) {
+        return std::vector<uint8_t>{
+            static_cast<uint8_t>((v >> 24) & 0xFF),
+            static_cast<uint8_t>((v >> 16) & 0xFF),
+            static_cast<uint8_t>((v >> 8) & 0xFF),
+            static_cast<uint8_t>(v & 0xFF)};
+    };
+    auto chunk = [&](const std::string& tag,
+                     const std::vector<uint8_t>& payload) {
+        std::vector<uint8_t> out = be32(static_cast<uint32_t>(payload.size()));
+        std::vector<uint8_t> tagged(tag.begin(), tag.end());
+        tagged.insert(tagged.end(), payload.begin(), payload.end());
+        out.insert(out.end(), tagged.begin(), tagged.end());
+        const uint32_t crc = static_cast<uint32_t>(
+            ::crc32(0L, tagged.data(), static_cast<uInt>(tagged.size())));
+        const auto crcBytes = be32(crc);
+        out.insert(out.end(), crcBytes.begin(), crcBytes.end());
+        return out;
+    };
+
+    std::vector<uint8_t> png = {0x89, 'P', 'N', 'G', '\r', '\n', 0x1A, '\n'};
+
+    std::vector<uint8_t> ihdr = be32(w);
+    const auto hh = be32(h);
+    ihdr.insert(ihdr.end(), hh.begin(), hh.end());
+    ihdr.insert(ihdr.end(), {8, 6, 0, 0, 0});  // 8-bit RGBA
+    const auto ihdrChunk = chunk("IHDR", ihdr);
+    png.insert(png.end(), ihdrChunk.begin(), ihdrChunk.end());
+
+    // One filter byte + 4 bytes/px per row, all zero (transparent black).
+    std::vector<uint8_t> raw(static_cast<size_t>(h) * (1 + 4 * w), 0);
+    uLongf destLen = compressBound(static_cast<uLong>(raw.size()));
+    std::vector<uint8_t> deflated(destLen);
+    // Return an empty fixture on compression failure rather than emitting a
+    // PNG with a truncated IDAT — callers fail fast (setIcon() rejects empty
+    // data), so the real cause surfaces instead of a confusing decode error.
+    if (compress2(deflated.data(), &destLen, raw.data(),
+                  static_cast<uLong>(raw.size()), 9) != Z_OK) {
+        return {};
+    }
+    deflated.resize(destLen);
+    const auto idat = chunk("IDAT", deflated);
+    png.insert(png.end(), idat.begin(), idat.end());
+
+    const auto iend = chunk("IEND", {});
+    png.insert(png.end(), iend.begin(), iend.end());
+    return png;
+}
+
+// Write a PNG fixture to disk and return the path, for CLI tests that pass
+// --icon rather than calling Package::setIcon() directly.
+inline std::string writePng(const std::string& path,
+                            uint32_t w = 256,
+                            uint32_t h = 256) {
+    const auto data = makePng(w, h);
+    std::ofstream out(path, std::ios::binary);
+    out.write(reinterpret_cast<const char*>(data.data()),
+              static_cast<std::streamsize>(data.size()));
+    return path;
+}
+
+} // namespace lgx_test