Files
loader/_docs/03_implementation/reviews/batch_01_review.md
T
Oleksandr Bezdieniezhnykh d244799f02 [AZ-182][AZ-184][AZ-187] Batch 1
Made-with: Cursor
2026-04-15 07:23:47 +03:00

2.9 KiB

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