Audit BLE protocol parity after Kable migration - #5
Merged
9thLevelSoftware merged 2 commits intoNov 29, 2025
Merged
Conversation
Document complete analysis comparing Kable-based KMP implementation against parent Nordic BLE repository. Identifies critical gaps including: - Missing 0x04 activation command (96-byte frame) - Missing 0x0A INIT/RESET command for fault recovery - Incomplete 0x4E Echo command parameters - Scan filter gaps for V-Trainer/Vitruvian device variants Includes detailed byte-by-byte protocol verification, implementation recommendations with code samples, and risk assessment for fixes.
- Add workoutmachineappfree as authoritative protocol source - CRITICAL: Identify mode value discrepancy (web app: 0-4 vs build: 0,2,3,4,6) - Clarify 0x4F (official deobfuscated) vs 0x04 (web app) command difference - Update gap analysis priorities based on actual protocol source - Add hardware verification test matrix for mode values
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
9thLevelSoftware
deleted the
claude/audit-ble-kable-migration-01NTfPgGZgcEkZF5sXuUcEXd
branch
November 29, 2025 22:05
9thLevelSoftware
added a commit
that referenced
this pull request
Feb 16, 2026
… issue **Issues Fixed:** - #2: Race condition - SingleExerciseScreen now awaits routine load via loadRoutineAsync() before ensureConnection(), preventing PR weight fetch from racing with onConnected callback - #4: Keypad input - CompactNumberPicker.ios.kt sanitizes input and handles parse failure by keeping current value instead of max (242 lbs) - #6: Unilateral weight - ActiveSessionEngine detects single-cable exercises and uses maxOf(loadA, loadB) instead of totalLoad/2 - #7: Mode persistence - SqlDelightWorkoutRepository.parseProgramMode() handles unprefixed legacy mode strings ("Pump" vs "Program:Pump") **Diagnostic Logging for Issue #5:** Added WEIGHT_DEBUG logging at 5 points to trace weight value flow: - iOS CompactNumberPicker scroll handler - JustLiftScreen picker onValueChange - JustLiftScreen params update - ActiveSessionEngine set completion - SetSummaryCard display calculation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9thLevelSoftware
added a commit
that referenced
this pull request
Apr 19, 2026
…atedAt Resolves audit items #5 and #10 (.planning/audit/06-dto-drift-matrix.md). - ExternalActivityAckDto now carries `localId`, `serverId`, and `updatedAt` (all nullable defaults so older response payloads still decode) so mobile can reconcile server-canonical metadata (notably the `updated_at` timestamp used to seed LWW convergence in Phase 3) onto local rows after push. - PortalSyncPushResponse.externalActivityIds marked @deprecated; prefer externalActivityKeys which now carries full ack metadata. Retained for one release for older adapters. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8 tasks
9thLevelSoftware
added a commit
that referenced
this pull request
Apr 19, 2026
…370) * cursor: iOS CI, Android FGS, BLE/sync, and Koin iOS @throws Made-with: Cursor * fix(ios,ci,android): repair Koin iOS symbol, Supabase xcconfig, and release guard - iOS: Swift was calling `KoinInitKt.doInitKoin()`, but the new `@Throws` entrypoint lives in `shared/iosMain/.../KoinInitIos.kt`, so Kotlin/Native exports it as `KoinInitIosKt.doInitKoin()`. Without this fix the iOS app fails to compile. Switched the Swift call (with `try`) and updated the KoinInit.kt doc comment + iosApp/README accordingly. - CI: The three iOS workflows wrote `Supabase.xcconfig` without escaping `//`, which xcconfig treats as an inline comment. A secret like `https://xxx.supabase.co` was being truncated to `https:` at build time, breaking Supabase in TestFlight/Release. Apply the same `/$()/` escape already used by the committed `.example` template via `sed 's|//|/$()/|g'` to both URL and anon-key fields. - Android: Replaced `findByName("assembleRelease").forEach { doFirst }` with `tasks.matching { ... }.configureEach { doFirst }` so the explicit-versionCode guard attaches regardless of when AGP registers the release tasks (findByName at configuration time silently no-ops if the task isn't registered yet). Made-with: Cursor * fix(mobile): beta audit hardening + DTO drift Phase 1 Bundles the in-progress beta audit remediation work with the first phase of the DTO drift remediation plan (.planning/audit/06-dto-drift-matrix.md, resolved items #2 wire-contract portion, #3, #4). Beta audit hardening: - .gitignore (C1): cover Supabase.xcconfig in any VitruvianPhoenix subdir so a stray sibling file cannot leak the anon key - PortalAuthRepository (C6): explicit failure messages for social sign-in stubs; consumers can surface a clear reason instead of string-sniffing - GamificationViewModel (H): surface load errors via StateFlow instead of swallowing silently; log via kermit - RepCounterFromMachine (Issue #163): remove repCountersNeedBaseline baseline suppression; first post-reset rep now counts immediately, matching expected UX after BLE reconnect - SafeWordListener (android/ios): hardening of listener lifecycle - FakePortalApiClient: test double alignment - New common-test suites covering asymmetry threshold, error classification, portal mapping weight, pull pagination, push limits, token refresh, sync backoff, sync failure cap, velocity zone boundaries, workout phase round trip - Supabase.xcconfig + xcodeproj: secret wiring adjustment - androidApp/build.gradle.kts: build config cleanup DTO drift Phase 1: - PullWorkoutSessionDto (#2): add nullable `notes` field so session-level notes round-trip from portal. Mobile persistence is tracked separately (Phase 3.5) because WorkoutSession is per-exercise while portal sessions are per-workout; notes need a SessionNotes side-table keyed on routineSessionId rather than a straight column addition. - PortalSetDto (#3): documentation clarifying that prType/prPhase/prVolume are SEND-ONLY PR derivation hints consumed by the portal into personal_records; they are intentionally not persisted on the sets table. - PortalRepTelemetryDto (#4): update `cable` doc comment to reflect the canonical "A"|"B" wire format (A = left actuator, B = right actuator). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(mobile): extend ExternalActivityAckDto with localId/serverId/updatedAt Resolves audit items #5 and #10 (.planning/audit/06-dto-drift-matrix.md). - ExternalActivityAckDto now carries `localId`, `serverId`, and `updatedAt` (all nullable defaults so older response payloads still decode) so mobile can reconcile server-canonical metadata (notably the `updated_at` timestamp used to seed LWW convergence in Phase 3) onto local rows after push. - PortalSyncPushResponse.externalActivityIds marked @deprecated; prefer externalActivityKeys which now carries full ack metadata. Retained for one release for older adapters. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mobile): cap parityIds at MAX_PARITY_IDS=500 with tail-window fallback Resolves audit item #7 (.planning/audit/06-dto-drift-matrix.md). Server-side mobile-sync-pull now enforces HTTP 413 when any parity list exceeds 500 entries (replacing the prior silent-empty behavior). This commit adds matching client-side capping so legitimate power-user accounts do not get rejected. - `SyncConfig.MAX_PARITY_IDS = 500` new constant matching the server cap. - `capParity()` helper in `runPullLoop` truncates each parity list to the last MAX_PARITY_IDS entries when oversized; the pull then falls back to server-side lastSync delta semantics for the older tail, and local dedupe handles overlap. Logs a warning at WARN level when truncation occurs so power users' accounts are observable in telemetry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(mobile): populate updatedAt on push DTOs + decode LWW rejections Phase 3.2 client-side companion to the portal LWW push handler swap. Adds optional `updatedAt: String?` (ISO 8601) to the three shared-edit push DTOs that will flow through LWW RPCs on the server: - PortalWorkoutSessionDto - PortalRoutineSyncDto - PortalTrainingCycleSyncDto PortalSyncAdapter populates each with `epochToIso8601(currentTimeMillis())` on every build so the server's LWW gate receives a real monotonic timestamp. When mobile domain models start tracking per-row updated_at end-to-end (Phase 3.3), swap the adapter to emit domain values instead. PortalSyncPushResponse now decodes a nullable-default `rejections: SyncRejectionsDto` field (with nested SyncRejectionDto entries), so the mobile SyncManager can log LWW rejections and trigger a repair pull. Older server responses without the field still decode because of the empty default. The server side uses NOW() when `updatedAt` is absent (pre-Phase-3.2 clients), so existing mobile builds continue to work unmodified while updated builds participate in LWW immediately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(mobile): self-cap + sliding-window client rate limit for sync Phase 4.2 of the DTO drift remediation plan. Resolves audit item #9. A misbehaving retry loop or a user with a huge local history should fail fast locally instead of wasting Edge Function invocations for payloads the server will reject with HTTP 413/429. SyncConfig additions (mirror server-side caps at mobile-sync-push): - MAX_SESSIONS_PER_BATCH = 10000 - MAX_ROUTINES_PER_BATCH = 10000 - MAX_CYCLES_PER_BATCH = 10000 (aligned with audit #6) - MAX_TELEMETRY_PER_BATCH = 10000 - MAX_PAYLOAD_BYTES = 9_500_000 (500 KiB below 10 MiB server cap) - PUSH_RATE_LIMIT_PER_MIN = 10 - PULL_RATE_LIMIT_PER_MIN = 20 - RATE_LIMIT_WINDOW_MS = 60_000 PortalApiClient.pushPortalPayload now validates the array caps and the serialized payload size BEFORE opening the HTTP connection. On violation it returns `Result.failure(PortalApiException(...))` with a clear message. The serialized bytes are reused (not re-encoded) for the request body so there is no double-JSON cost. New ClientRateLimiter is an in-memory sliding-window limiter keyed by operation ("push" / "pull"). Internal state is a Mutex-guarded deque of recent attempt timestamps. SyncManager's push/pull entry points call `tryAcquire()` at the very top and surface a 429-coded PortalApiException when denied. In-memory scope only for this phase — a persistent SQLDelight-backed variant is noted as a follow-up for when process restarts during a retry storm would defeat the window. SyncManager gains an optional `rateLimiter: ClientRateLimiter = ClientRateLimiter()` constructor parameter so existing DI wiring keeps working unchanged; tests can supply a `ClientRateLimiter(windowMillis = ...)` with a reset hook for deterministic assertions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(mobile): persist portal session notes via SessionNotes side-table Phase 3.5 of the DTO drift remediation plan. Closes the mobile persistence gap left from Phase 1.1 of audit item #2 (portal session-level notes round-trip on the wire but were not stored locally). The mobile WorkoutSession model is per-exercise (one portal session expands into N mobile rows keyed by routineSessionId), so adding a `notes` column to WorkoutSession would duplicate state across rows. Instead this phase introduces a single-row-per-portal-session side-table. Schema (migration 26.sqm + main VitruvianDatabase.sq): - CREATE TABLE SessionNotes( routineSessionId TEXT PRIMARY KEY, notes TEXT, updatedAt INTEGER ) - INDEX idx_session_notes_updated_at on (updatedAt) for delta queries. - upsertSessionNotesLww query gates writes on `excluded.updatedAt >= SessionNotes.updatedAt` (NULL stored treated as older). Uses the same LWW semantics shipping in Phase 3.1 RPCs server-side so portal edits from a newer device win-by-timestamp. - getSessionNotes / selectSessionNotesForIds for read paths. Repository surface: - SyncRepository gains `mergeSessionNotes(notes: Map<String, SessionNotesEntry>)` with a default no-op so existing test fakes keep compiling. - SqlDelightSyncRepository wraps the upsert in a single transaction. SyncManager pull merge: - Extracts non-blank session notes from the pull response into a Map keyed on `routineSessionId` (== portal session id). - Persists outside the main atomic merge so a notes-table failure is non-fatal — sessions remain consistent even if the side-table write fails. Logged at WARN. - updatedAt sourced from `PullWorkoutSessionDto.startedAt` parsed via kotlin.time.Instant. When the wire DTO eventually carries a real `updatedAt` field (Phase 3.3 mobile-side LWW), swap to that. Build: - SQLDelight schema version bumped 26 → 27 to match the new migration. UI surface (session detail screen rendering of notes) is intentionally out of scope for this commit and tracked as follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mobile): SQLDelight 3.18-compatible SessionNotes upsert + manifest entry Two CI failures from PR #370: 1. SQLDelight gen failed: the dialect (sqlite_3_18) does not support INSERT ... ON CONFLICT DO UPDATE. Replaced the single `upsertSessionNotesLww` query with two SQLite-3.18-compatible queries (`selectSessionNotesUpdatedAt` + `upsertSessionNotes` using `INSERT OR REPLACE`) and moved the LWW gate into Kotlin (`SqlDelightSyncRepository.mergeSessionNotes`) where it runs inside the same transaction as the read. 2. iOS Schema Sync Check: SessionNotes missing from SchemaManifest.kt. Added the SchemaTableOperation entry plus a SchemaIndexOperation for `idx_session_notes_updated_at` so the iOS-side schema bootstrap stays parity-aligned with the SQLDelight schema. Also fixed `SELECT ... WHERE routineSessionId IN ?;` to the SQLDelight named-list form `WHERE routineSessionId IN :ids` (the bare `?` is not a valid bind for IN in SQLDelight). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mobile): import currentTimeMillis in PortalSyncAdapter Phase 3.2 commit (3494794) added `currentTimeMillis()` calls for the new `updatedAt = epochToIso8601(currentTimeMillis())` LWW timestamps at lines 196 / 510 / 600 but missed the import. CI compile failed with 'Unresolved reference currentTimeMillis' on linux because the function lives in `com.devil.phoenixproject.util.KmpUtils`. Add the import. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(mobile): assert parity ids capped at MAX_PARITY_IDS, not uncapped Test was authored with the pre-Phase-4.1 behavior (mobile sends uncapped knownEntityIds; server silently returns empty for >500). Phase 4.1 (audit item #7) inverted that contract: server now enforces HTTP 413 at 500, mobile capParity() truncates to the most recent 500 entries via takeLast(). Test renamed + assertion flipped to match new contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.