mirror of
https://github.com/azaion/loader.git
synced 2026-04-22 10:56:33 +00:00
[AZ-182][AZ-184][AZ-187] Batch 1
Made-with: Cursor
This commit is contained in:
@@ -0,0 +1,60 @@
|
||||
# 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
|
||||
Reference in New Issue
Block a user