Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt (1)

111-128: ⚠️ Potential issue | 🟠 Major

Keep messageId consistent between mms:received and mms:downloaded events.

The mms:received event (line 100) uses message.messageId ?: message.transactionId, but mms:downloaded uses the provider row ID (passed from MmsContentObserver.processMmsDownloaded as mmsId.toString()). Webhook consumers cannot reliably correlate these two stages of the same MMS if the identifiers differ.

Consider surfacing the original MMS header identifiers (messageId, transactionId) from MmsContentReader so the downloaded event can use consistent identification.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt`
around lines 111 - 128, The mms:downloaded event is currently using the provider
row ID instead of the same identifier used by mms:received, so update the MMS
pipeline to surface original MMS header IDs from MmsContentReader (messageId and
transactionId) through MmsContentObserver.processMmsDownloaded into
ReceiverService and use the same selection logic (message.messageId ?:
message.transactionId) when constructing the WebHookEvent.MmsDownloaded /
MmsDownloadedPayload (the messageId field) so consumers can correlate
mms:received and mms:downloaded reliably.

app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt (1)

82-97: ⚠️ Potential issue | 🟠 Major

Don't advance the high-water mark past a failed MMS.

Line 95 updates storage.mmsLastProcessedID = mmsId unconditionally after processing, even when an exception occurs. A transient failure (e.g., provider read error) will permanently skip that MMS since the mark advances past it.

Consider only advancing the mark on successful processing, or changing the callback pattern to allow soft failures.

Suggested fix

         cursor.use { c ->
             while (c.moveToNext()) {
                 val mmsId = c.getLong(0)
                 try {
                     processMmsDownloaded(mmsId)
+                    storage.mmsLastProcessedID = mmsId
                 } catch (e: Exception) {
                     logsService.insert(
                         LogEntry.Priority.ERROR,
                         MODULE_NAME,
                         "Failed processing downloaded MMS (id=$mmsId)",
                         mapOf("mmsId" to mmsId)
                     )
+                    break  // Stop processing; retry this MMS on next onChange
                 }
-                storage.mmsLastProcessedID = mmsId
             }
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt`
around lines 82 - 97, The loop in MmsContentObserver currently advances the
high-water mark unconditionally (storage.mmsLastProcessedID = mmsId) even when
processMmsDownloaded(mmsId) throws; change it so the mark is updated only after
successful processing — e.g., move the storage.mmsLastProcessedID assignment
into the try block after processMmsDownloaded returns (or set it only when no
exception is caught), and keep the logsService.insert call in the catch so
failures do not advance the mark and the MMS can be retried.

app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentReader.kt (1)

87-91: ⚠️ Potential issue | 🟡 Minor

Handle text/plain parts with file-backed storage.

When a content://mms/part row has ct='text/plain' and _data is non-null, the text should be read from the part URI stream rather than the text column. The current code only checks the text column, causing the body to be empty when text is stored file-backed (atypical but observed in some OEM implementations).

Suggested fix

                 if (contentType == "text/plain") {
-                    val text = c.getString(2)
+                    val text = c.getString(2)?.takeIf { it.isNotBlank() }
+                        ?: readTextFromPart(resolver, partId)
                     if (!text.isNullOrBlank()) {
                         bodyParts.add(text)
                     }

Add a helper function:

private fun readTextFromPart(resolver: ContentResolver, partId: Long): String? {
    return try {
        resolver.openInputStream(Uri.parse("content://mms/part/$partId"))?.use { input ->
            input.bufferedReader().readText().takeIf { it.isNotBlank() }
        }
    } catch (_: Exception) {
        null
    }
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentReader.kt`
around lines 87 - 91, The text/plain branch in MmsContentReader.kt currently
only reads c.getString(2) and skips file-backed parts; add a helper function
readTextFromPart(resolver: ContentResolver, partId: Long): String? (as
suggested) that opens content://mms/part/{partId} and returns buffered text or
null, then in the contentType == "text/plain" handling check the _data/part id
from the cursor (use the part id column) and if the text from c.getString(2) is
null/blank but _data is present, call readTextFromPart(resolver, partId) and add
its result to bodyParts if non-blank so file-backed text parts are included.

app/src/main/java/me/capcom/smsgateway/modules/settings/Module.kt (1)

63-67: Consider using single {} instead of factory {} for StateStorage.

The factory {} binding creates a new StateStorage instance on each injection. While this works correctly (all instances share the same underlying SharedPreferences), using single {} would be more efficient and consistent with the intent of having a single state manager.

♻️ Suggested change

-    factory {
+    single {
         StateStorage(
             PreferencesStorage(get(), "receiver")
         )
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/modules/settings/Module.kt` around
lines 63 - 67, Replace the Koin binding for StateStorage which currently uses
factory { StateStorage(PreferencesStorage(get(), "receiver")) } with a singleton
binding: change the provider to single { StateStorage(PreferencesStorage(get(),
"receiver")) } so the StateStorage is created once and reused; update the
binding where factory is used to refer to StateStorage and ensure no other code
relies on multiple instances.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

app/src/main/assets/api/swagger.json (2)

4-32: Missing required array for non-nullable properties.

Comparing with the Kotlin Attachment class, partId, contentType, and size are non-nullable, so they should be marked as required in the schema. This ensures API consumers relying on the OpenAPI spec for validation or code generation get accurate type information.

📝 Proposed fix to add required fields

           "size": {
             "description": "Size in bytes, 0 if unknown.",
             "type": "integer"
           }
         },
+        "required": [
+          "partId",
+          "contentType",
+          "size"
+        ],
         "type": "object"
       },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/assets/api/swagger.json` around lines 4 - 32, The
MmsDownloadedAttachment schema is missing a required array for non-nullable
properties; update the "MmsDownloadedAttachment" object to include "required":
["partId","contentType","size"] so that partId, contentType, and size are
declared required (matching the Kotlin Attachment class) and ensure the types
remain unchanged under the properties block.

---

33-68: Missing required array and consider adding maxItems constraint.

1. Based on the Kotlin MmsDownloadedPayload class, attachments and receivedAt are non-nullable and should be marked as required in the nested object definition.

2. The static analysis tool flags the attachments array lacking a maxItems constraint. While MMS messages are carrier-size-limited, adding a reasonable upper bound (e.g., 100) improves API predictability for consumers.

📝 Proposed fix to add required fields and maxItems

               "attachments": {
                 "description": "Metadata for non-text MMS parts, including optional Base64 content.",
                 "items": {
                   "$ref": "#/components/schemas/MmsDownloadedAttachment"
                 },
+                "maxItems": 100,
                 "type": "array"
               },
               "body": {
                 "description": "Aggregated text content of the MMS message.",
                 "nullable": true,
                 "type": "string"
               },
               "receivedAt": {
                 "description": "The timestamp when the MMS message was received.",
                 "format": "date-time",
                 "type": "string"
               },
               "subject": {
                 "description": "Message subject line.",
                 "nullable": true,
                 "type": "string"
               }
             },
+            "required": [
+              "attachments",
+              "receivedAt"
+            ],
             "type": "object"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/assets/api/swagger.json` around lines 33 - 68, The nested object
schema for MmsDownloadedPayload is missing a required array and attachments
lacks a maxItems constraint: update the MmsDownloadedPayload schema (the inner
object under "allOf") to include a "required" array listing "attachments" and
"receivedAt", and add a sensible "maxItems" (e.g., 100) to the "attachments"
array definition to bound its length and match the Kotlin non-nullability of
attachments and receivedAt.

app/src/main/assets/api/swagger.json (2)

158-185: Consider aligning schema name with the new title.

The schema name remains SmsEventPayload while the title was updated to MessageEventPayload to reflect broader usage beyond SMS. This inconsistency could cause confusion for API consumers. Consider renaming the schema definition to MessageEventPayload for consistency, though this would be a breaking change for existing clients.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/assets/api/swagger.json` around lines 158 - 185, The schema key
is still named SmsEventPayload while its title was changed to
MessageEventPayload; rename the schema definition from SmsEventPayload to
MessageEventPayload and update all references to
components.schemas.SmsEventPayload (e.g., $ref values) across the OpenAPI spec
so they point to components.schemas.MessageEventPayload, keeping the "title":
"MessageEventPayload" and existing properties unchanged; if you need to avoid
breaking clients instead add an aliased schema (keep SmsEventPayload as a $ref
to the new MessageEventPayload) and document the deprecation.

---

45-50: Consider adding maxItems constraint for the attachments array.

Static analysis flagged that the attachments array lacks a maxItems constraint. While MMS messages are carrier-size-limited (typically 1-3 MB total), adding a reasonable upper bound (e.g., "maxItems": 100) provides input validation at the API layer.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/assets/api/swagger.json` around lines 45 - 50, The attachments
array schema (property "attachments" referencing
"#/components/schemas/MmsDownloadedAttachment") needs an explicit upper bound to
validate input; update the Swagger/OpenAPI fragment for "attachments" to include
a "maxItems" (e.g., 100) so the array cannot exceed the reasonable size limit
for MMS payloads and keep the existing "type": "array" and "$ref" intact.