# Code Review Report

**Batch**: 1 (AZ-182, AZ-184, AZ-187) — loader repo only
**Date**: 2026-04-15
**Verdict**: PASS_WITH_WARNINGS

**Note**: AZ-183 (Resources Table & Update API) is scoped to the admin API repository and was excluded from this batch. A mock /get-update endpoint was added to the loader's e2e mock API. See cross-repo notes below.

## Spec Compliance

All 16 acceptance criteria across 3 tasks are satisfied with corresponding tests.

| Task | ACs | Covered | Status |
|------|-----|---------|--------|
| AZ-182 TPM Security Provider | 6 | 6/6 | All pass (AC-2 skips without swtpm) |
| AZ-184 Resumable Download Manager | 5 | 5/5 | All pass (8/8 unittest) |
| AZ-187 Device Provisioning Script | 5 | 5/5 | All pass (5/5 pytest) |

## Findings

| # | Severity | Category | File:Line | Title |
|---|----------|----------|-----------|-------|
| 1 | Medium | Style | src/download_manager.py:113 | Union syntax inconsistency |
| 2 | Low | Style | tests/test_download_manager.py:9-11 | Redundant sys.path manipulation |
| 3 | Low | Scope | AZ-183 | Out-of-repo task excluded |

### Finding Details

**F1: Union syntax inconsistency** (Medium / Style)
- Location: `src/download_manager.py:113`
- Description: Uses `Callable[[], requests.Session] | None` syntax while the rest of the project uses `Optional[...]` (e.g., `main.py` uses `Optional[str]`)
- Suggestion: Use `Optional[Callable[[], requests.Session]]` for consistency
- Task: AZ-184

**F2: Redundant sys.path manipulation** (Low / Style)
- Location: `tests/test_download_manager.py:9-11`
- Description: `sys.path.insert(0, str(SRC))` is redundant — `pytest.ini` already sets `pythonpath = src`
- Suggestion: Remove the sys.path block; tests run via pytest which handles the path
- Task: AZ-184

**F3: Out-of-repo task excluded** (Low / Scope)
- Location: AZ-183 task spec
- Description: AZ-183 (Resources Table & Update API) targets the admin API repository, not the loader. Excluded from this batch.
- Suggestion: Implement in the admin API workspace. A mock /get-update endpoint was added to `e2e/mocks/mock_api/app.py` for loader e2e tests.

## Cross-Task Consistency

- AZ-182 and AZ-184 both add loader-side capabilities; no interface conflicts
- AZ-187 standalone provisioning script has no coupling issues
- Mock /get-update endpoint response format (cdnUrl, sha256, encryptionKey) aligns with AZ-184 download manager expectations

## Cross-Repo Notes (AZ-183)

AZ-183 requires implementation in the **admin API repository** (`admin/`):
- Resources table migration (resource_name, dev_stage, architecture, version, cdn_url, sha256, encryption_key, size_bytes, created_at)
- POST /get-update endpoint: accepts device's current versions + architecture + dev_stage, returns only newer resources
- Server-side memory cache invalidated on CI/CD publish
- Internal endpoint for CI/CD to publish new resource versions
- encryption_key column must be encrypted at rest
- Response must include encryption_key only over HTTPS with valid JWT