Skip to content

feat: implement /api/update/readiness endpoint + structured update ch…#352

Open
eva57gr wants to merge 2 commits intoLight-Heart-Labs:mainfrom
eva57gr:pr-02-dashboard-api-update-readiness
Open

feat: implement /api/update/readiness endpoint + structured update ch…#352
eva57gr wants to merge 2 commits intoLight-Heart-Labs:mainfrom
eva57gr:pr-02-dashboard-api-update-readiness

Conversation

@eva57gr
Copy link
Contributor

@eva57gr eva57gr commented Mar 17, 2026

…ecks

Copy link
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: REQUEST CHANGES

Blocking: Bare except Exception in _check_compatibility_status()

CLAUDE.md: "Never except Exception: return." This catches everything and returns a dict instead of crashing. Narrow to except OSError (the only realistic failure mode beyond the already-caught TimeoutExpired), or remove.

Blocking: Synchronous subprocess.run in async endpoint

Blocks the event loop for up to 30s. Every other subprocess call in this file uses asyncio.create_subprocess_exec. Use that or asyncio.to_thread().

Blocking: Duplicated script resolution

New _resolve_update_script_for_readiness() has a different candidate list than trigger_update(). Readiness could report "available" while POST /api/update returns 501. Unify into one shared function.

Non-blocking:

  • Dead model_dump / isinstance coercion block — get_version() always returns dict, remove the duck-typing
  • datetime.now(timezone.utc).isoformat() + "Z" repeated 6+ times — extract helper
  • Tests are well-structured, good coverage

🤖 Reviewed with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants