Skip to content

fix(models): close mmproj integrity gaps (catch-up move + size floor)#23

Merged
fl4p merged 1 commit into
mainfrom
fix/mmproj-integrity-gaps
Jul 4, 2026
Merged

fix(models): close mmproj integrity gaps (catch-up move + size floor)#23
fl4p merged 1 commit into
mainfrom
fix/mmproj-integrity-gaps

Conversation

@fl4p

@fl4p fl4p commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Follow-up to #18. A post-merge adversarial review found two real defects in the mmproj GGUF integrity validation that shipped with #18.

Bugs

  1. Catch-up guard kept corrupt sidecars. The mmproj sidecar can complete natively before the onComplete listener registers, so there's a "catch-up guard" path. On move-failure that path only checked RNFS.exists(target) — if a corrupt-but-present file sat at the target it was silently kept as the vision projector. The onComplete handler already validated properly; only the catch-up path was missed. Both paths now share reconcileMmProjMoveFailure() (read-only validate → reuse if valid, downgrade to text-only if not, never delete), so they can't drift again.

  2. Size check bypassed when expected size is unknown. mmProjFileValid/checkMmProjExists only compared size if (expectedSize). On crash-restore the reconstructed download can have mmProjFileSize = 0/undefined, so the size check was skipped and a 0-byte/truncated stub with an inconclusive magic read passed as valid. Added an absolute MIN_MMPROJ_FILE_SIZE (1 KB) floor independent of expectedSize, mirroring the existing MIN_GGUF_FILE_SIZE floor in llmSafetyChecks.

Tests

  • catch-up path now downgrades-without-deleting when the present target is corrupt (bad GGUF magic)
  • catch-up path rejects a sub-1KB stub when expectedSize is unknown
  • Full modelManager + parallelMmproj + integration/models suites pass (343 tests); tsc and lint clean.

Two defects in the mmproj GGUF integrity validation (#18), found in post-merge review:

1. The catch-up guard (mmproj completes natively before the onComplete listener
   registers) only checked target existence on move-failure, silently keeping a
   corrupt-but-present sidecar as a vision projector. It now runs the same
   read-only validation as the onComplete handler. Both paths share a new
   reconcileMmProjMoveFailure() so they can't drift again.

2. mmProjFileValid / checkMmProjExists skipped the size check when expectedSize
   was 0/undefined (reachable on crash-restore), so a 0-byte/truncated stub with
   an inconclusive magic read passed as valid. Added an absolute MIN_MMPROJ_FILE_SIZE
   (1 KB) floor, independent of expectedSize, mirroring llmSafetyChecks.

Tests: catch-up now downgrades-without-deleting on a corrupt present target, and
rejects a sub-1KB stub when expectedSize is unknown.
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.

1 participant