ImpTune/.planning/phases/06-wire-icon-intunewin/06-RESEARCH.md

# Phase 6: Wire Icon into .intunewin Export — Research

**Researched:** 2026-04-10
**Domain:** Python file I/O, Peewee ORM query, .intunewin staging pipeline
**Confidence:** HIGH

---

<phase_requirements>
## Phase Requirements

| ID | Description | Research Support |
|----|-------------|-----------------|
| PKG-04 | User can upload a custom PNG icon for Intune app display (256x256, max 750KB) | Icon upload (icons.py + Icon model) is already complete. This phase wires the stored icon into the .intunewin build by copying it into the tmpdir before `build_intunewin()` is called. |
</phase_requirements>

---

## Summary

Phase 5 implemented icon upload and storage (`icons.py`, `Icon` model, SHA256-addressed
file under `DATA_DIR/icons/`). The `get_intunewin_package()` endpoint in `packages.py`
already builds a `.intunewin` file from a `TemporaryDirectory` staging area. The gap is
that the endpoint never queries the `Icon` model, so the PNG never lands in the tmpdir and
is never encrypted into the inner ZIP.

The work is minimal and mechanical: one `Icon.get_or_none()` lookup, one `shutil.copy2()`
(or equivalent `open/write`) into `tmpdir`, and an integration test that verifies the icon
is present inside the exported package. No schema changes, no new dependencies.

**Primary recommendation:** Query `Icon.get_or_none(Icon.printer == printer_id)` in
`get_intunewin_package()`, and if an icon exists copy it into `tmpdir` as `icon.png`
before calling `build_intunewin()`. Add one integration test that uploads an icon and
verifies `icon.png` appears inside the `.intunewin` inner ZIP.

---

## Standard Stack

### Core (already in place — no new installs)

| Library | Version | Purpose | Why Standard |
|---------|---------|---------|--------------|
| peewee | 3.x | ORM query for Icon record | Already used throughout; `Icon.get_or_none()` follows established pattern |
| Python shutil / pathlib | stdlib | Copy icon file from ICONS_DIR into tmpdir | Zero-dep, already used in codebase |
| Pillow | current | Not needed here (already validated on upload) | Upload already guarantees PNG 256x256 |
| zipfile | stdlib | Read inner ZIP of .intunewin to assert icon presence in tests | Already used in test_packages.py |

**Installation:** None required. All dependencies exist.

---

## Architecture Patterns

### Existing tmpdir staging pattern (packages.py lines 132-149)

```python
with tempfile.TemporaryDirectory(prefix="imptune_") as tmpdir:
    # Write scripts
    with open(os.path.join(tmpdir, "install.ps1"), "w", encoding="utf-8") as f:
        f.write(install_script)
    # ... uninstall.ps1, detect.ps1 ...

    # Extract driver ZIP into tmpdir/drivers/
    drivers_subdir = os.path.join(tmpdir, "drivers")
    os.makedirs(drivers_subdir, exist_ok=True)
    with zipfile.ZipFile(driver_zip_path, "r") as driver_zf:
        driver_zf.extractall(drivers_subdir)

    # Build .intunewin
    build_intunewin(tmpdir, "install.ps1", output_path)
```

The icon copy slots in immediately after driver extraction, before `build_intunewin()`.

### Pattern: icon lookup + conditional copy

```python
import shutil
from imptune.db.models import Icon

# Inside get_intunewin_package(), after driver validation:
icon_record = Icon.get_or_none(Icon.printer == printer_id)
if icon_record is not None:
    icon_src = os.path.join(cfg.ICONS_DIR, icon_record.sha256)
    icon_dst = os.path.join(tmpdir, "icon.png")
    shutil.copy2(icon_src, icon_dst)
```

**Key decision:** Copy is conditional — printers without an uploaded icon still export
successfully. `shutil.copy2` preserves metadata and is the idiomatic stdlib copy call.

### Icon filename in tmpdir

Use `"icon.png"` as the fixed destination name regardless of `original_filename`. This is
predictable for the test assertion and matches Intune's expectation for app icons (a
well-known filename in the package root).

### Established ORM access pattern (from project decisions)

- Use `Model.get_or_none()` (not `Model.get()`) — avoids `DoesNotExist` exception
- Test assertions use `list(Model.select().where(...))` not `Model.get()` — avoids Peewee
  cursor-caching issue with re-init'd DBs across tests (Phase 03 decision)

### Anti-Patterns to Avoid

- **Raising 422 when icon is absent:** Icon is optional. The export must succeed without one.
- **Storing icon in a subdirectory:** `build_intunewin` walks `source_dir` recursively; any
  placement works, but root-level `icon.png` is simplest and most predictable.
- **Re-validating the PNG in packages.py:** Validation already happened at upload time. Do
  not import Pillow into packages.py; just copy the bytes.

---

## Don't Hand-Roll

| Problem | Don't Build | Use Instead | Why |
|---------|-------------|-------------|-----|
| File copy into tmpdir | Custom open/read/write loop | `shutil.copy2()` | Handles edge cases, one line |
| Icon existence check | Filesystem path probe | `Icon.get_or_none()` | Source of truth is the DB record, not filesystem |
| Verify icon in .intunewin | Manual ZIP decryption | Read inner ZIP via `zipfile` after decoding (see test pattern below) | The test can open the outer ZIP and then the inner ZIP without decryption using stored bytes |

---

## Common Pitfalls

### Pitfall 1: ICONS_DIR not `cfg.ICONS_DIR`

**What goes wrong:** Hardcoding `DATA_DIR + "/icons"` instead of reading `cfg.ICONS_DIR`.
**Why it happens:** `cfg.ICONS_DIR` was added in Phase 5 (config.py line 13); easy to miss.
**How to avoid:** Always use `cfg.ICONS_DIR` — monkeypatched by `tmp_data_dir` fixture in tests.
**Warning signs:** Integration test writes icon to `cfg.ICONS_DIR` but packages.py reads
from a hardcoded path — icon silently missing in package.

### Pitfall 2: Icon copy placed after `build_intunewin()` call

**What goes wrong:** Icon is written to tmpdir but after `build_intunewin()` already ran;
not included in the inner ZIP.
**How to avoid:** Icon copy must come BEFORE `build_intunewin(tmpdir, ...)` call.

### Pitfall 3: Test verifying icon in outer ZIP instead of inner ZIP

**What goes wrong:** The `.intunewin` outer ZIP contains only `IntuneWinPackage/Contents/
IntunePackage.intunewin` (encrypted blob) and `Detection.xml`. `icon.png` is NOT a member
of the outer ZIP; it is packed into the inner ZIP (the plaintext content ZIP). A test that
opens the outer ZIP and checks `namelist()` for `icon.png` will always fail.
**How to avoid:** The integration test must decrypt (or use a pre-decryption approach — see
Code Examples below) to verify icon presence. The simplest approach: use a test fixture
that calls `build_intunewin` directly against a tmpdir so the inner ZIP can be inspected
before encryption, OR read the Detection.xml and re-derive key material from a
test-seeded build. Given current architecture uses random keys, the cleanest test strategy
is:
  a) Make a real HTTP request through the test client to get the `.intunewin` bytes
  b) Use `build_intunewin`'s known structure: the inner ZIP is the AES-encrypted payload
     — cannot open it directly without the key.
  c) **Best approach:** Test that the icon file is present in `tmpdir` staging BEFORE
     `build_intunewin()` is called by exposing a helper, OR introduce a
     `build_intunewin` that accepts a pre-built inner ZIP for testability.
  d) **Pragmatic approach that avoids refactoring:** In the integration test, directly
     call the packages.py function with a monkeypatched `build_intunewin` that records
     what was in `tmpdir` instead of encrypting. This is the pattern to use.

**Recommended test strategy:** Monkeypatch `build_intunewin` to capture the staging
directory contents, then assert `icon.png` is present among the staged files. This is
simpler than decrypting the output and avoids coupling the test to the encryption
implementation.

### Pitfall 4: `printer_id` vs ORM instance in Icon query

**What goes wrong:** `Icon.get_or_none(Icon.printer == printer)` (ORM instance) vs
`Icon.get_or_none(Icon.printer == printer_id)` (integer). Both work in Peewee, but the
integer form is more explicit and consistent with how icons.py deletes records
(`Icon.delete().where(Icon.printer == printer_id)`).
**How to avoid:** Use integer `printer_id` (the local variable from `_get_printer_and_driver`
returns the printer ORM object; extract `.id` from it or use it directly — Peewee resolves
FK equality either way, but be consistent with existing codebase style).

---

## Code Examples

### Icon lookup and conditional copy (insert inside `get_intunewin_package`)

```python
# Source: derived from icons.py pattern + packages.py TemporaryDirectory pattern
import shutil
from imptune.db.models import Icon

# After extracting driver ZIP, before build_intunewin():
icon_record = Icon.get_or_none(Icon.printer == printer.id)
if icon_record is not None:
    icon_src = os.path.join(cfg.ICONS_DIR, icon_record.sha256)
    if os.path.isfile(icon_src):
        shutil.copy2(icon_src, os.path.join(tmpdir, "icon.png"))
```

### Integration test — icon presence in staged files (monkeypatch approach)

```python
# Source: pattern established by test_packages.py + test_icon_upload.py

def test_intunewin_includes_icon(client, setup_printer_with_driver, tmp_data_dir, monkeypatch):
    """Icon PNG is included in .intunewin staging directory."""
    import io
    import os
    from PIL import Image
    from imptune.db.models import Icon
    import imptune.config as cfg
    import imptune.api.packages as pkg_module

    printer, _ = setup_printer_with_driver

    # Create and upload icon
    img = Image.new("RGBA", (256, 256), color="red")
    buf = io.BytesIO()
    img.save(buf, format="PNG")
    png_data = buf.getvalue()

    resp = client.post(
        f"/printers/{printer.id}/icon",
        files={"file": ("icon.png", io.BytesIO(png_data), "image/png")},
    )
    assert resp.status_code == 200

    # Capture staging contents via monkeypatched build_intunewin
    staged_files = []

    def fake_build(source_dir, setup_file, output_path):
        staged_files.extend(os.listdir(source_dir))
        # Write a minimal valid file so endpoint can read it
        with open(output_path, "wb") as f:
            f.write(b"FAKE")

    monkeypatch.setattr(pkg_module, "build_intunewin", fake_build)

    resp = client.get(f"/printers/{printer.id}/packages/intunewin")
    assert resp.status_code == 200
    assert "icon.png" in staged_files
```

### Test: export succeeds with no icon (icon-free baseline)

```python
def test_intunewin_without_icon_succeeds(client, setup_printer_with_driver, monkeypatch):
    """Export succeeds even when no icon has been uploaded."""
    import imptune.api.packages as pkg_module
    import os

    printer, _ = setup_printer_with_driver

    def fake_build(source_dir, setup_file, output_path):
        with open(output_path, "wb") as f:
            f.write(b"FAKE")

    monkeypatch.setattr(pkg_module, "build_intunewin", fake_build)

    resp = client.get(f"/printers/{printer.id}/packages/intunewin")
    assert resp.status_code == 200
```

---

## State of the Art

| Old Approach | Current Approach | When Changed | Impact |
|--------------|------------------|--------------|--------|
| icon stored as column in Printer | Separate `Icon` model with FK, SHA256-addressed on disk | Phase 5 | Clean separation; icon is optional; no schema migration needed here |
| build_intunewin reads arbitrary files | build_intunewin walks entire tmpdir | Phase 5 | Any file placed in tmpdir automatically included in inner ZIP |

---

## Open Questions

1. **Should missing icon file on disk (orphaned DB record) be silently skipped or 422?**
   - What we know: `icon_record` can exist in DB but file might be absent (disk corruption,
     manual cleanup). Current `icons.py` does not handle this case either.
   - Recommendation: Silent skip (don't copy if `os.path.isfile(icon_src)` is False) —
     consistent with the optional nature of icons and avoids breaking exports for stale records.

2. **Is there any Intune-specific requirement for the icon filename or location inside .intunewin?**
   - What we know: The `.intunewin` package is a deployment container; Intune does NOT
     read the icon from within the .intunewin file. The icon is set separately in the
     Intune portal during app creation. The PKG-04 requirement says "included in the
     .intunewin package" as a convenience/archive — not as an Intune-interpreted artifact.
   - Confidence: MEDIUM — based on reverse-engineering of .intunewin format. The
     requirement text "Intune displays it as the app icon" describes the end-state in
     Intune, not a technical requirement for the .intunewin binary.
   - Recommendation: Include the icon in the package root as `icon.png` for user
     convenience (they can extract it when uploading manually to Intune portal).

---

## Validation Architecture

### Test Framework

| Property | Value |
|----------|-------|
| Framework | pytest (detected: pytest.ini or pyproject.toml implicit) |
| Config file | none detected — runs via `python -m pytest` |
| Quick run command | `python -m pytest tests/test_packages.py -x -q` |
| Full suite command | `python -m pytest tests/ -q` |

### Phase Requirements -> Test Map

| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|--------|----------|-----------|-------------------|-------------|
| PKG-04 | Icon PNG is copied into .intunewin staging dir when icon exists | integration | `python -m pytest tests/test_packages.py::TestIntunewinDownload -x -q` | Partial — test_packages.py exists but no icon-in-package test yet |
| PKG-04 | Export succeeds when no icon uploaded | integration | `python -m pytest tests/test_packages.py::TestIntunewinDownload -x -q` | Partial — baseline export tests exist |

### Sampling Rate

- **Per task commit:** `python -m pytest tests/test_packages.py -x -q`
- **Per wave merge:** `python -m pytest tests/ -q`
- **Phase gate:** Full suite green before `/gsd:verify-work`

### Wave 0 Gaps

- [ ] `tests/test_packages.py` — add `test_intunewin_includes_icon` test (monkeypatch `build_intunewin`)
- [ ] `tests/test_packages.py` — add `test_intunewin_without_icon_succeeds` test

*(Existing test infrastructure covers the rest — no new files or fixtures needed)*

---

## Sources

### Primary (HIGH confidence)

- Direct code reading: `imptune/api/packages.py` — full `get_intunewin_package()` implementation
- Direct code reading: `imptune/api/icons.py` — Icon upload, SHA256 storage path, `cfg.ICONS_DIR`
- Direct code reading: `imptune/db/models.py` — `Icon` model, FK to `Printer`, `sha256` field
- Direct code reading: `imptune/config.py` — `ICONS_DIR = str(Path(DATA_DIR) / "icons")`
- Direct code reading: `imptune/generators/intunewin_builder.py` — `build_intunewin` walks entire `source_dir`
- Direct code reading: `tests/conftest.py` — `tmp_data_dir` monkeypatches `cfg.ICONS_DIR`
- Direct code reading: `tests/test_packages.py` — existing test patterns (fixtures, monkeypatch style)
- Direct code reading: `.planning/STATE.md` — Phase 5 decisions on ICONS_DIR, Icon model, shutil pattern

### Secondary (MEDIUM confidence)

- Intune .intunewin format: icon is NOT read from within the package by Intune — icons are
  set in the Intune portal. Including it in the package is for user convenience only.
  (Based on reverse-engineering notes in `intunewin_builder.py` comments + svrooij.io reference)

---

## Metadata

**Confidence breakdown:**
- Standard stack: HIGH — all libraries already in codebase, no new dependencies
- Architecture: HIGH — single code location change (packages.py), clear insertion point
- Pitfalls: HIGH — inner-ZIP test trap verified by reading intunewin_builder.py structure
- Test strategy: HIGH — monkeypatch pattern already established in existing test suite

**Research date:** 2026-04-10
**Valid until:** Stable — no moving parts (stdlib + existing codebase only)