Open
Conversation
WalkthroughAdds an Inbox API with persistence and DTOs, registers InboxRoutes under /inbox (GET list with paging/filters, POST /refresh), extends receiver/export flow with a triggerWebhooks flag to conditionally emit webhooks, and updates OpenAPI schemas/tags accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant WebService as WebService
participant InboxRoutes as InboxRoutes
participant IncomingService as IncomingService
participant Repository as Repository
participant Database as Database
Client->>WebService: GET /inbox (auth, query)
WebService->>InboxRoutes: dispatch (authenticated)
InboxRoutes->>IncomingService: count(type, from, to)
IncomingService->>Repository: count(...)
Repository->>Database: SQL COUNT query
Database-->>Repository: countResult
Repository-->>IncomingService: countResult
InboxRoutes->>IncomingService: select(type, from, to, limit, offset)
IncomingService->>Repository: select(...)
Repository->>Database: SQL SELECT (ORDER BY createdAt DESC, id DESC LIMIT/OFFSET)
Database-->>Repository: rows
Repository-->>IncomingService: List<IncomingMessage>
IncomingService-->>InboxRoutes: List<InboxMessage>
InboxRoutes-->>Client: 200 OK (body List, header X-Total-Count)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/assets/api/swagger.json (2)
881-926: Missingrequiredarray for schema properties.The
smsgateway.IncomingMessageschema doesn't specify which fields are required. Based on theIncomingMessageResponsedata class,id,type,sender,contentPreview, andcreatedAtare non-nullable and should be marked as required.Proposed fix
"type": "object" - } + }, + "required": [ + "id", + "type", + "sender", + "contentPreview", + "createdAt" + ]🤖 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 881 - 926, The OpenAPI schema smsgateway.IncomingMessage is missing a required array; update the schema for smsgateway.IncomingMessage to include a "required" property listing the non-nullable fields from the IncomingMessageResponse data class (id, type, sender, contentPreview, createdAt) so those properties are marked as mandatory in the spec.
2069-2082: MissingX-Total-Countheader documentation.The route implementation sets an
X-Total-Countresponse header for pagination, but the OpenAPI spec doesn't document this header in the 200 response. This should be documented for API consumers.Proposed fix
"200": { "content": { "application/json": { "schema": { "items": { "$ref": "#/components/schemas/smsgateway.IncomingMessage" }, "type": "array" } } }, - "description": "A list of incoming messages" + "description": "A list of incoming messages", + "headers": { + "X-Total-Count": { + "description": "Total number of messages matching the filter criteria", + "schema": { + "type": "integer" + } + } + } },🤖 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 2069 - 2082, The OpenAPI 200 response for the endpoint returning "smsgateway.IncomingMessage" arrays is missing the documented X-Total-Count pagination header; update the 200 response object in the swagger.json for this route to include a "headers" entry with "X-Total-Count" (type: integer, format: int64) and a brief description (total number of items for pagination) so API consumers see the header alongside the array schema referencing smsgateway.IncomingMessage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt`:
- Around line 301-302: The code currently uses IncomingMessageType.valueOf(...)
which throws IllegalArgumentException for invalid strings and leads to 500;
update the parsing of the query parameter (the local variable `type` in
MessagesRoutes.kt) to safely map the incoming string to an IncomingMessageType
(e.g., use a safe lookup like enumValues<IncomingMessageType>().firstOrNull {
it.name == input } or runCatching) and if the value is null/invalid return a 400
Bad Request with a clear error message instead of letting the exception
propagate.
---
Nitpick comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 881-926: The OpenAPI schema smsgateway.IncomingMessage is missing
a required array; update the schema for smsgateway.IncomingMessage to include a
"required" property listing the non-nullable fields from the
IncomingMessageResponse data class (id, type, sender, contentPreview, createdAt)
so those properties are marked as mandatory in the spec.
- Around line 2069-2082: The OpenAPI 200 response for the endpoint returning
"smsgateway.IncomingMessage" arrays is missing the documented X-Total-Count
pagination header; update the 200 response object in the swagger.json for this
route to include a "headers" entry with "X-Total-Count" (type: integer, format:
int64) and a brief description (total number of items for pagination) so API
consumers see the header alongside the array schema referencing
smsgateway.IncomingMessage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f4e0fc3-1874-4caa-a4ba-1b5f5f3cb068
📒 Files selected for processing (9)
app/src/main/assets/api/swagger.jsonapp/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/GetIncomingMessagesResponse.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/IncomingMessageResponse.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
capcom6
force-pushed
the
codex/plan-api-access-for-incoming-messages
branch
from
6915810 to
5c59f8f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 2194-2203: The OpenAPI spec for the 404 response on
/messages/inbox/{id} declares a JSON body type smsgateway.ErrorResponse but the
route handler in MessagesRoutes.kt returns HttpStatusCode.NotFound with no body;
update one side to match the other. Either change the handler that serves
/messages/inbox/{id} to include and serialize an instance of
smsgateway.ErrorResponse when returning 404, or modify the swagger.json entry
for that operation to use a 404 response with no content (remove the
application/json/schema reference and make it an empty response). Ensure the
change references the route in MessagesRoutes.kt and the schema name
smsgateway.ErrorResponse so the contract and implementation are consistent.
- Around line 881-926: The schema for smsgateway.IncomingMessage currently marks
all properties optional but the API always returns id, type, sender,
contentPreview, and createdAt; update the smsgateway.IncomingMessage definition
to include a "required" array listing those five fields (id, type, sender,
contentPreview, createdAt) so generated SDK types and validation reflect the
actual contract; ensure you update any related references such as
IncomingMessageResponse if it reuses this schema.
- Around line 2070-2082: The OpenAPI response for the /messages/inbox 200
response is missing the X-Total-Count header that MessagesRoutes.kt sets; update
the swagger response object for that operation to include a headers entry named
"X-Total-Count" with a numeric/integer schema (e.g., type: integer, format:
int64) and a short description such as "Total number of items available" so
pagination clients can read the total count; locate the 200 response block that
returns an array of smsgateway.IncomingMessage and add the headers definition
there to mirror the runtime behavior in MessagesRoutes.kt.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac3c5a8c-811c-49f3-bb35-6c73033eea6e
📒 Files selected for processing (9)
app/src/main/assets/api/swagger.jsonapp/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/GetIncomingMessagesResponse.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/domain/IncomingMessageResponse.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
✅ Files skipped from review due to trivial changes (3)
- app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/GetIncomingMessagesResponse.kt
- app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt
- app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/IncomingMessageResponse.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.kt
- app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt (3)
103-103: Unnecessary cast.The
as GetIncomingMessagesResponsecast is redundant sincemessages.map { it.toDomain() }already returnsList<InboxMessage>which matches the typealias.Proposed fix
- call.respond(messages.map { it.toDomain() } as GetIncomingMessagesResponse) + call.respond(messages.map { it.toDomain() })🤖 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/localserver/routes/InboxRoutes.kt` at line 103, The cast on the respond call is unnecessary; replace the expression so call.respond receives the mapped list directly instead of casting. Specifically, in the route handler change the call.respond invocation that currently uses messages.map { it.toDomain() } as GetIncomingMessagesResponse to call.respond(messages.map { it.toDomain() }), leaving the functions/methods involved (messages, toDomain(), call.respond, GetIncomingMessagesResponse typealias) unchanged.
106-122: Remove commented-out code or track it as a TODO.This commented-out
get("{id}")endpoint should either be removed to keep the codebase clean, or if it's planned for implementation, add a TODO/issue reference.🤖 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/localserver/routes/InboxRoutes.kt` around lines 106 - 122, Remove the large commented-out get("{id}") endpoint block (including the requireScope(AuthScopes.InboxRead) check, incomingMessagesService.getById call and the call.respond error handling) to keep the codebase clean; if this route is intended to be implemented later, replace the commented code with a single TODO comment referencing the issue/ticket number and a brief note like "implement inbox get by id" so intent is tracked instead of leaving dead commented code in the repository.
124-136: Endpoint path and scope naming could be clearer.The
POST "update"endpoint reusesAuthScopes.MessagesExportand the path name "update" doesn't clearly convey that this is an inbox sync/refresh operation without webhook triggers. Consider a more descriptive path like"sync"or"refresh"and potentially a dedicated scope if the permission model differs.That said, the
triggerWebhooks = falseis correct here for the "silent" DB sync use case.🤖 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/localserver/routes/InboxRoutes.kt` around lines 124 - 136, The POST route currently defined as post("update") uses AuthScopes.MessagesExport but is intended to perform a silent inbox sync (receiverService.export(context, request.period, false)), so rename the route to a clearer path like "sync" or "refresh" in the InboxRoutes declaration and, if your permission model differs for inbox syncs, replace AuthScopes.MessagesExport with a dedicated scope (e.g., AuthScopes.InboxSync or MessagesInboxSync) used in the requireScope check; update any related client callers/tests to use the new path and scope name so they remain consistent with the new intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Line 103: The cast on the respond call is unnecessary; replace the expression
so call.respond receives the mapped list directly instead of casting.
Specifically, in the route handler change the call.respond invocation that
currently uses messages.map { it.toDomain() } as GetIncomingMessagesResponse to
call.respond(messages.map { it.toDomain() }), leaving the functions/methods
involved (messages, toDomain(), call.respond, GetIncomingMessagesResponse
typealias) unchanged.
- Around line 106-122: Remove the large commented-out get("{id}") endpoint block
(including the requireScope(AuthScopes.InboxRead) check,
incomingMessagesService.getById call and the call.respond error handling) to
keep the codebase clean; if this route is intended to be implemented later,
replace the commented code with a single TODO comment referencing the
issue/ticket number and a brief note like "implement inbox get by id" so intent
is tracked instead of leaving dead commented code in the repository.
- Around line 124-136: The POST route currently defined as post("update") uses
AuthScopes.MessagesExport but is intended to perform a silent inbox sync
(receiverService.export(context, request.period, false)), so rename the route to
a clearer path like "sync" or "refresh" in the InboxRoutes declaration and, if
your permission model differs for inbox syncs, replace AuthScopes.MessagesExport
with a dedicated scope (e.g., AuthScopes.InboxSync or MessagesInboxSync) used in
the requireScope check; update any related client callers/tests to use the new
path and scope name so they remain consistent with the new intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e00bb84d-f0f9-4304-bc21-7f414a980712
📒 Files selected for processing (9)
app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/EventsReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt
- app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 106-122: Uncomment and reintroduce the GET "{id}" route handler so
the inbox-by-id endpoint is exposed: restore the block that calls
requireScope(AuthScopes.InboxRead), reads call.parameters["id"], returns
BadRequest if missing, invokes incomingMessagesService.getById(id), returns
NotFound when null, wraps the service call in a try/catch to respond
InternalServerError with the exception message on failure, and finally
call.respond(message.toDomain()) on success; use the existing identifiers
(get("{id}"), requireScope, incomingMessagesService.getById, call.respond,
message.toDomain) so behavior matches the original commented implementation.
- Around line 86-87: Replace all API responses that interpolate raw exception
text (the mapOf("message" to "Failed to count incoming messages: ${e.message}"),
and the similar mapOf usages at the other two handlers) with a generic
client-safe message (e.g., mapOf("message" to "Failed to count incoming
messages")) and ensure the full exception is logged internally (e.g.,
logger.error("Failed to count incoming messages", e)) so debug info is retained
but not leaked to clients; do this for the three handlers in InboxRoutes.kt that
currently return ${e.message}.
- Around line 57-62: The route currently silently defaults when
DateTimeParser.parseIsoDateTime returns null; change it to reject malformed ISO
datetimes: for both the "from" and "to" query params, if the param is present
but DateTimeParser.parseIsoDateTime(...) yields null, immediately call
call.respond(HttpStatusCode.BadRequest, "Invalid 'from' datetime") or "...'to'
datetime" (as appropriate) and return from the handler; otherwise use the parsed
.time value (and if the param is absent keep the existing default behavior).
Reference the query handling around the 'from' and 'to' variables and
DateTimeParser.parseIsoDateTime to implement the check and early 400 response.
- Around line 36-37: The code currently uses IncomingMessageType.valueOf(it)
when parsing the "type" query parameter which throws IllegalArgumentException
for unknown values; change parsing in the route handler so invalid enum names
produce a 400 response instead of throwing: attempt to map the string to
IncomingMessageType (using a safe lookup or catching IllegalArgumentException
around IncomingMessageType.valueOf), and on failure call
call.respond(HttpStatusCode.BadRequest, ...) with a clear message; preserve the
existing behavior when the parameter is absent or empty and mirror the
validation style used for limit/offset/deviceId in this route.
- Around line 126-127: The
call.receive(PostMessagesInboxExportRequest).validate() happens before the try
so deserialization/validation exceptions escape and become 500s; move the
deserialization and validation into the try block
(call.receive<PostMessagesInboxExportRequest>() and
PostMessagesInboxExportRequest.validate()) and catch expected client errors
(e.g., Ktor's ContentTransformationException or your validation exception type)
in the catch to respond with HttpStatusCode.BadRequest and an explanatory
message via call.respond; keep the existing handling for other exceptions to
preserve 500 behavior.
- Around line 83-84: The current route handlers in InboxRoutes.kt catch
Throwable (the catch (e: Throwable) blocks) which swallows
CancellationException; change those exception handlers to first catch
CancellationException and rethrow it (catch (e: CancellationException) { throw e
}) and then catch Exception (or Throwable excluding CancellationException) to
handle and respond as before—update both occurrences around the call.respond
blocks so coroutine cancellations propagate correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1d415f3-0d74-43e7-b26a-5629fde7b960
📒 Files selected for processing (2)
app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt (2)
139-155:⚠️ Potential issue | 🟠 Major
GET /messages/inbox/{id}is still not exposed.The
{id}handler is fully commented out, so the by-id inbox endpoint remains unavailable despite being part of this feature scope/API contract.🤖 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/localserver/routes/InboxRoutes.kt` around lines 139 - 155, Uncomment and re-enable the GET by-id route in InboxRoutes.kt: restore the get("{id}") handler so the /messages/inbox/{id} endpoint is exposed, keep the authorization check requireScope(AuthScopes.InboxRead), retrieve the id from call.parameters, call incomingMessagesService.getById(id) and handle null with HttpStatusCode.NotFound, preserve the try/catch that returns HttpStatusCode.InternalServerError with the exception message on failure, and finally respond with message.toDomain(); ensure identifiers get("{id}"), requireScope, AuthScopes.InboxRead, incomingMessagesService.getById, and message.toDomain() are used exactly as in the diff.
115-118:⚠️ Potential issue | 🟠 MajorAvoid returning raw exception messages to API clients.
Lines 117, 129, and 181 include
${e.message}in response bodies, which can leak internals. Return a generic client-safe message instead (and keep full details only in server logs).Suggested fix
- mapOf("message" to "Failed to count incoming messages: ${e.message}") + mapOf("message" to "Failed to count incoming messages") ... - mapOf("message" to "Failed to retrieve incoming messages: ${e.message}") + mapOf("message" to "Failed to retrieve incoming messages") ... - mapOf("message" to "Failed to refresh inbox: ${e.message}") + mapOf("message" to "Failed to refresh inbox")Also applies to: 127-130, 179-182
🤖 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/localserver/routes/InboxRoutes.kt` around lines 115 - 118, The route handlers in InboxRoutes are returning raw exception messages to clients (calls to call.respond that build mapOf("message" to "... ${e.message}")), which can leak internals; remove the interpolation of e.message in those responses and instead return a generic client-safe message (e.g., "Failed to count incoming messages" / "Failed to list incoming messages" / "Failed to delete incoming message") while logging the full exception server-side (logger.error("Failed to ...", e) or similar) so the status codes remain HttpStatusCode.InternalServerError but no raw exception text is exposed to API clients.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 47-48: The route currently silently defaults non-numeric query
parameters by using toIntOrNull for limit/offset; instead, in the InboxRoutes
handler check whether "limit" or "offset" are present in
call.request.queryParameters and if present but toIntOrNull returns null respond
with HttpStatusCode.BadRequest (or throw BadRequestException) with a clear
message; otherwise parse them to Int (use the parsed values or default only when
the parameter is absent). Update the logic around the existing limit and offset
parsing in InboxRoutes.kt (the expressions using
call.request.queryParameters["limit"]?.toIntOrNull() and
["offset"]?.toIntOrNull()) to implement this validation and call.respond/error
path.
- Around line 160-170: The catch chain around
call.receive(PostMessagesInboxExportRequest).validate() is currently swallowing
CancellationException; add an explicit catch for
java.util.concurrent.CancellationException (or
kotlinx.coroutines.CancellationException) before the generic catch (e:
Exception) to rethrow it immediately so cancellation propagates properly; update
the try/catch in the route that calls call.receive and
PostMessagesInboxExportRequest.validate to catch CancellationException and
rethrow, then keep the existing IllegalArgumentException and Exception handlers
unchanged.
---
Duplicate comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 139-155: Uncomment and re-enable the GET by-id route in
InboxRoutes.kt: restore the get("{id}") handler so the /messages/inbox/{id}
endpoint is exposed, keep the authorization check
requireScope(AuthScopes.InboxRead), retrieve the id from call.parameters, call
incomingMessagesService.getById(id) and handle null with
HttpStatusCode.NotFound, preserve the try/catch that returns
HttpStatusCode.InternalServerError with the exception message on failure, and
finally respond with message.toDomain(); ensure identifiers get("{id}"),
requireScope, AuthScopes.InboxRead, incomingMessagesService.getById, and
message.toDomain() are used exactly as in the diff.
- Around line 115-118: The route handlers in InboxRoutes are returning raw
exception messages to clients (calls to call.respond that build mapOf("message"
to "... ${e.message}")), which can leak internals; remove the interpolation of
e.message in those responses and instead return a generic client-safe message
(e.g., "Failed to count incoming messages" / "Failed to list incoming messages"
/ "Failed to delete incoming message") while logging the full exception
server-side (logger.error("Failed to ...", e) or similar) so the status codes
remain HttpStatusCode.InternalServerError but no raw exception text is exposed
to API clients.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5362ce8a-cae9-4b8a-a8fa-09ddeeae1132
📒 Files selected for processing (5)
app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.kt
- app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
capcom6
force-pushed
the
codex/plan-api-access-for-incoming-messages
branch
from
e7bb92a to
63921a5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt (4)
47-48:⚠️ Potential issue | 🟡 MinorReject malformed
limit/offsetinstead of silently defaulting.Non-numeric values like
limit=abcsilently default to50/0instead of returning 400, unlike thetypeparameter which properly rejects invalid values. For consistency and to surface client errors:Suggested fix
- val limit = call.request.queryParameters["limit"]?.toIntOrNull() ?: 50 - val offset = call.request.queryParameters["offset"]?.toIntOrNull() ?: 0 + val limitRaw = call.request.queryParameters["limit"] + val limit = if (limitRaw == null) { + 50 + } else { + limitRaw.toIntOrNull() ?: run { + call.respond(HttpStatusCode.BadRequest, mapOf("message" to "Invalid limit")) + return@get + } + } + val offsetRaw = call.request.queryParameters["offset"] + val offset = if (offsetRaw == null) { + 0 + } else { + offsetRaw.toIntOrNull() ?: run { + call.respond(HttpStatusCode.BadRequest, mapOf("message" to "Invalid offset")) + return@get + } + }🤖 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/localserver/routes/InboxRoutes.kt` around lines 47 - 48, The handler in InboxRoutes.kt silently defaults non-numeric query params for limit/offset (the variables limit and offset) instead of rejecting them; change the parsing so that if call.request.queryParameters["limit"] or ["offset"] is present but toIntOrNull() returns null, the route immediately responds with HttpStatusCode.BadRequest (400) and a clear error message about the invalid parameter, otherwise use the parsed Int or the default (50 for limit, 0 for offset); implement this validation where limit and offset are currently computed in the route handler so malformed values are rejected consistently with the type parameter handling.
160-171:⚠️ Potential issue | 🟠 MajorPreserve cancellation semantics in the request parsing catch chain.
At line 168,
catch (e: Exception)will swallowCancellationExceptionfromcall.receive(...), breaking cooperative cancellation. Add an explicitCancellationExceptioncatch before the broadExceptioncatch, consistent with the pattern used at lines 176-177.Suggested fix
val request = try { call.receive<PostMessagesInboxExportRequest>().validate() + } catch (e: CancellationException) { + throw e } catch (e: IllegalArgumentException) { call.respond( HttpStatusCode.BadRequest, mapOf("message" to (e.message ?: "Invalid request")) ) return@post } catch (e: Exception) { call.respond(HttpStatusCode.BadRequest, mapOf("message" to "Invalid request body")) return@post }🤖 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/localserver/routes/InboxRoutes.kt` around lines 160 - 171, The catch block that handles exceptions from call.receive in InboxRoutes.kt currently catches Exception and will swallow CancellationException, breaking coroutine cancellation; update the try/catch in the POST handler around call.receive<PostMessagesInboxExportRequest>().validate() to add an explicit catch (e: CancellationException) { throw e } (or rethrow) before the generic catch (e: Exception) so that cancellation is preserved; keep the existing IllegalArgumentException and generic BadRequest handling for validate() failures.
139-155:⚠️ Potential issue | 🟠 Major
GET /{id}inbox route is commented out (feature gap).The Swagger spec documents
GET /messages/inbox/{id}(lines 2155-2238), but this handler is disabled. Either uncomment and implement properly (addressing thecatch (e: Throwable)issue within), or remove the endpoint from the Swagger spec to avoid API contract mismatch.🤖 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/localserver/routes/InboxRoutes.kt` around lines 139 - 155, The GET "{id}" inbox route in InboxRoutes.kt is commented out causing an API mismatch; uncomment and restore the handler for the endpoint that uses requireScope(AuthScopes.InboxRead), call.parameters["id"], incomingMessagesService.getById(id) and message.toDomain(), but replace the broad catch (e: Throwable) with specific error handling: validate and return BadRequest if id is missing, return NotFound when getById returns null, catch and handle known service exceptions (e.g., DataAccessException or custom service errors) mapping them to appropriate HttpStatusCodes and include error.message in the response, and let unexpected exceptions bubble to a global error handler (or convert them to InternalServerError with sanitized message); ensure the response uses call.respond(message.toDomain()) and update the OpenAPI/Swagger only if you choose to remove the endpoint instead of restoring it.
117-117:⚠️ Potential issue | 🟠 MajorDo not expose raw exception messages in API responses.
Lines 117, 129, and 181 return
${e.message}directly to clients, which can leak internal details (DB/query/runtime internals). Use generic messages and log the full exception server-side.Suggested fix
- mapOf("message" to "Failed to count incoming messages: ${e.message}") + mapOf("message" to "Failed to count incoming messages") ... - mapOf("message" to "Failed to retrieve incoming messages: ${e.message}") + mapOf("message" to "Failed to retrieve incoming messages") ... - mapOf("message" to "Failed to refresh inbox: ${e.message}") + mapOf("message" to "Failed to refresh inbox")Also applies to: lines 129, 181
🤖 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/localserver/routes/InboxRoutes.kt` at line 117, The handlers in InboxRoutes are returning raw exception text (e.message) in API responses (e.g., mapOf("message" to "Failed to count incoming messages: ${e.message}")), which can leak internals; replace those responses with generic error strings like "Failed to count incoming messages" / "Failed to fetch incoming messages" / "Failed to process request" and log the full exception server-side using the existing logger (e.g., logger.error("Failed to count incoming messages", e)) in the catch blocks for the affected endpoints so clients get no raw exception details while the server retains the full stack trace for debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 2155-2238: The OpenAPI spec includes a documented path GET
/messages/inbox/{id} but the corresponding route handler in InboxRoutes.kt is
commented out; either remove this path from app/src/main/assets/api/swagger.json
or re-enable and wire up the route handler in InboxRoutes.kt (the commented GET
handler for the inbox message by id) so the spec matches runtime behavior; if
you re-enable, ensure the handler name/path, parameter schema (id as string),
response schemas (smsgateway.IncomingMessage and smsgateway.ErrorResponse) and
security (ApiAuth, JWTAuth) in the swagger entry match the actual controller
implementation and update/add tests accordingly.
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 187-205: The DTO InboxMessage and its mapper
IncomingMessage.toDomain() are missing the optional subscriptionId field
declared in the Swagger schema; add a nullable subscriptionId: String? property
to the InboxMessage data class and map it in the toDomain() function (e.g.,
subscriptionId = subscriptionId) so the runtime response shape matches
smsgateway.IncomingMessage, or alternatively remove subscriptionId from the API
schema — update either InboxMessage or the schema to keep them in sync.
---
Duplicate comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 47-48: The handler in InboxRoutes.kt silently defaults non-numeric
query params for limit/offset (the variables limit and offset) instead of
rejecting them; change the parsing so that if
call.request.queryParameters["limit"] or ["offset"] is present but toIntOrNull()
returns null, the route immediately responds with HttpStatusCode.BadRequest
(400) and a clear error message about the invalid parameter, otherwise use the
parsed Int or the default (50 for limit, 0 for offset); implement this
validation where limit and offset are currently computed in the route handler so
malformed values are rejected consistently with the type parameter handling.
- Around line 160-171: The catch block that handles exceptions from call.receive
in InboxRoutes.kt currently catches Exception and will swallow
CancellationException, breaking coroutine cancellation; update the try/catch in
the POST handler around
call.receive<PostMessagesInboxExportRequest>().validate() to add an explicit
catch (e: CancellationException) { throw e } (or rethrow) before the generic
catch (e: Exception) so that cancellation is preserved; keep the existing
IllegalArgumentException and generic BadRequest handling for validate()
failures.
- Around line 139-155: The GET "{id}" inbox route in InboxRoutes.kt is commented
out causing an API mismatch; uncomment and restore the handler for the endpoint
that uses requireScope(AuthScopes.InboxRead), call.parameters["id"],
incomingMessagesService.getById(id) and message.toDomain(), but replace the
broad catch (e: Throwable) with specific error handling: validate and return
BadRequest if id is missing, return NotFound when getById returns null, catch
and handle known service exceptions (e.g., DataAccessException or custom service
errors) mapping them to appropriate HttpStatusCodes and include error.message in
the response, and let unexpected exceptions bubble to a global error handler (or
convert them to InternalServerError with sanitized message); ensure the response
uses call.respond(message.toDomain()) and update the OpenAPI/Swagger only if you
choose to remove the endpoint instead of restoring it.
- Line 117: The handlers in InboxRoutes are returning raw exception text
(e.message) in API responses (e.g., mapOf("message" to "Failed to count incoming
messages: ${e.message}")), which can leak internals; replace those responses
with generic error strings like "Failed to count incoming messages" / "Failed to
fetch incoming messages" / "Failed to process request" and log the full
exception server-side using the existing logger (e.g., logger.error("Failed to
count incoming messages", e)) in the catch blocks for the affected endpoints so
clients get no raw exception details while the server retains the full stack
trace for debugging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ad4896f0-60d6-4768-b1be-9b74d895e7e2
📒 Files selected for processing (13)
app/src/main/assets/api/swagger.jsonapp/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/EventsReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
✅ Files skipped from review due to trivial changes (2)
- app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
- app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt
🚧 Files skipped from review as they are similar to previous changes (6)
- app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
- app/src/main/java/me/capcom/smsgateway/modules/receiver/EventsReceiver.kt
- app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.kt
- app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/assets/api/swagger.json (1)
1645-1650: ClarifydeviceIdbehavior in docs to match runtime validation.At runtime (
InboxRoutes.ktLines 34-137),deviceIdis validated to equal the current device ID, not used as a cross-device filter. The current description is too generic.📝 Suggested wording update
{ - "description": "Device ID", + "description": "Optional device ID; if provided, it must match the current device ID", "in": "query", "name": "deviceId", "schema": { "type": "string" } }🤖 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 1645 - 1650, Update the OpenAPI parameter description for the "deviceId" query parameter in swagger.json to match runtime behavior: state that the parameter is validated to equal the current device's ID (not used to filter across devices) and that requests with a different deviceId will be rejected; locate the parameter by the name "deviceId" in the swagger.json and align the description with the validation logic implemented in InboxRoutes.kt (the deviceId equality check in the route handler).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 1747-1755: The OpenAPI spec declares a JSON body for the 202
response but the InboxRoutes handler uses call.respond(HttpStatusCode.Accepted)
with no body; update the swagger.json 202 response to be a no-content response
(remove the "application/json" schema under the 202 response or replace it with
an empty content/no-content response) so the documented contract matches the
runtime behavior of the InboxRoutes route that returns HttpStatusCode.Accepted.
---
Nitpick comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 1645-1650: Update the OpenAPI parameter description for the
"deviceId" query parameter in swagger.json to match runtime behavior: state that
the parameter is validated to equal the current device's ID (not used to filter
across devices) and that requests with a different deviceId will be rejected;
locate the parameter by the name "deviceId" in the swagger.json and align the
description with the validation logic implemented in InboxRoutes.kt (the
deviceId equality check in the route handler).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b36ef529-41df-430f-a743-5c3387ebaf5b
📒 Files selected for processing (1)
app/src/main/assets/api/swagger.json
capcom6
changed the title
capcom6
changed the title
capcom6
force-pushed
the
codex/plan-api-access-for-incoming-messages
branch
from
69d3759 to
8b9fdfa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/assets/api/swagger.json (1)
1645-1650: ClarifydeviceIdbehavior in query docs.Line 1645 currently says only “Device ID”, but the handler rejects values that do not match the current device (
InboxRoutes.ktlines 97-104). Please document that constraint to prevent client confusion.🤖 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 1645 - 1650, The swagger.json description for the query parameter "deviceId" is too vague; update its description to state that the handler (InboxRoutes.kt, the inbox route that validates device identity) will reject any deviceId that does not match the requesting user's current device, so clients must supply the current device's ID (or omit it if not applicable) and expect a 4xx error when supplying a non-matching value; modify the "description" string for the "deviceId" param to clearly document this constraint and expected failure behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 1735-1740: The OpenAPI spec references
smsgateway.MessagesExportRequest (which requires deviceId) for the
/inbox/refresh request but the handler InboxRoutes.kt expects
PostMessagesInboxExportRequest (with only since and until); fix by making the
spec and code agree: either change the swagger.json requestBody $ref at the
/inbox/refresh operation to point to the correct schema (e.g., the
PostMessagesInboxExportRequest schema) or update the handler to accept
smsgateway.MessagesExportRequest (including deviceId) and adjust related DTOs;
ensure you update the components/schemas entry and any generated clients
accordingly so the schema referenced by $ref matches the Kotlin class used by
InboxRoutes.kt.
---
Nitpick comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 1645-1650: The swagger.json description for the query parameter
"deviceId" is too vague; update its description to state that the handler
(InboxRoutes.kt, the inbox route that validates device identity) will reject any
deviceId that does not match the requesting user's current device, so clients
must supply the current device's ID (or omit it if not applicable) and expect a
4xx error when supplying a non-matching value; modify the "description" string
for the "deviceId" param to clearly document this constraint and expected
failure behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db65d012-80da-4844-a24e-855aa8362302
📒 Files selected for processing (1)
app/src/main/assets/api/swagger.json
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/assets/api/swagger.json (1)
1991-2011:⚠️ Potential issue | 🟠 MajorAlign
/inbox/refreshcontract with the actual handler payload and response.Line 1995 references
smsgateway.MessagesExportRequest(requiresdeviceId), butInboxRoutes.ktreceivesPostMessagesInboxExportRequest(since/untilonly).
Also, Lines 2004-2010 document a JSON202body, while the handler returns status-onlyAccepted.Proposed OpenAPI fix
"requestBody": { "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/smsgateway.MessagesExportRequest" + "$ref": "#/components/schemas/smsgateway.InboxRefreshRequest" } } }, @@ "202": { - "content": { - "application/json": { - "schema": { - "type": "object" - } - } - }, "description": "Inbox refresh request accepted" },+ "smsgateway.InboxRefreshRequest": { + "properties": { + "since": { + "description": "Start of the time range to refresh.", + "format": "date-time", + "type": "string" + }, + "until": { + "description": "End of the time range to refresh.", + "format": "date-time", + "type": "string" + } + }, + "required": ["since", "until"], + "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 1991 - 2011, The OpenAPI contract for /inbox/refresh is incorrect: replace the requestBody schema reference from smsgateway.MessagesExportRequest to the actual handler type smsgateway.PostMessagesInboxExportRequest (which contains only since/until), and update the 202 response to be a no-content/empty response (remove the application/json content/schema) so it reflects that the handler returns status-only Accepted; adjust description if needed to "Inbox refresh request accepted" without a response body.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 514-523: The schema entries for enum-backed fields
(smsgateway.IncomingMessage.type, smsgateway.Webhook.event,
smsgateway.HealthCheck.status, smsgateway.HealthResponse.status,
smsgateway.LogEntry.priority, smsgateway.SettingsMessages.limit_period,
smsgateway.SettingsMessages.processing_order,
smsgateway.SettingsMessages.sim_selection_mode) currently declare a local
"type": "object" alongside an allOf $ref to a string enum, creating an
unsatisfiable schema; fix by removing only the local "type": "object" property
from each of these referenced property objects so the type is inherited from the
referenced enum schema (leave description/example and the allOf/$ref intact).
---
Duplicate comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 1991-2011: The OpenAPI contract for /inbox/refresh is incorrect:
replace the requestBody schema reference from smsgateway.MessagesExportRequest
to the actual handler type smsgateway.PostMessagesInboxExportRequest (which
contains only since/until), and update the 202 response to be a no-content/empty
response (remove the application/json content/schema) so it reflects that the
handler returns status-only Accepted; adjust description if needed to "Inbox
refresh request accepted" without a response body.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a12fb16-4cfd-42f4-9fbb-728034796759
📒 Files selected for processing (1)
app/src/main/assets/api/swagger.json
capcom6
force-pushed
the
codex/plan-api-access-for-incoming-messages
branch
from
72cd597 to
7ab8a09
Compare
capcom6
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 1481-1547: Swagger documents POST /auth/token/refresh but there is
no handler in AuthRoutes.kt; either remove this path from swagger.json or
implement the refresh endpoint: add a route in AuthRoutes.kt for POST
"/auth/token/refresh" that accepts the refresh token input, calls your token
refresh logic (e.g. TokenService.refreshToken or create a
refreshToken(jti/refreshToken) method if missing), validates/refreshes
credentials, returns smsgateway.TokenResponse on success and proper
smsgateway.ErrorResponse for 401/403/500 cases, and ensure the route honors
JWTAuth/security requirements and uses the same response schema names as the
Swagger file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cbf0e9f-48c2-4f06-9463-17bc582d32ba
📒 Files selected for processing (5)
app/src/main/assets/api/swagger.jsonapp/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
Contributor
🤖 Pull request artifacts
|
capcom6
force-pushed
the
codex/plan-api-access-for-incoming-messages
branch
from
7ab8a09 to
ae33155
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (8)
app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt (4)
160-170:⚠️ Potential issue | 🟠 MajorRethrow
CancellationExceptionbefore the broad request-parsing catch.
call.receive(...)runs in Ktor's coroutine context, andcatch (e: Exception)here will also swallow cancellation. That breaks normal request cancellation semantics.Suggested fix
val request = try { call.receive<PostMessagesInboxExportRequest>().validate() + } catch (e: CancellationException) { + throw e } catch (e: IllegalArgumentException) { call.respond( HttpStatusCode.BadRequest, mapOf("message" to (e.message ?: "Invalid request")) )🤖 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/localserver/routes/InboxRoutes.kt` around lines 160 - 170, The request parsing try/catch in the post handler (where call.receive<PostMessagesInboxExportRequest>().validate() is used in InboxRoutes.kt) currently catches Exception and will swallow coroutine cancellations; add an explicit catch for CancellationException that rethrows (e.g., catch (e: CancellationException) { throw e }) before the broad catch (e: Exception) so cancellations propagate correctly, and ensure CancellationException is imported if needed.
47-48:⚠️ Potential issue | 🟡 MinorReject malformed
limit/offsetvalues instead of silently defaulting.If the client sends
limit=abcoroffset=abc, this handler falls back to50/0and hides a bad request. Return400when the parameter is present but unparsable.🤖 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/localserver/routes/InboxRoutes.kt` around lines 47 - 48, The handler currently uses call.request.queryParameters["limit"]?.toIntOrNull() ?: 50 and similarly for offset, which silently defaults when the client supplies a malformed value; change this to first retrieve the raw param strings from call.request.queryParameters (e.g., "limit" and "offset"), and if a param is present but toIntOrNull() returns null respond with HTTP 400 (Bad Request) with an explanatory message, otherwise use the parsed Int or the default (50/0). Update the logic around the existing limit/offset variables in the Inbox route handler to implement this validation and early response.
114-129:⚠️ Potential issue | 🟠 MajorDo not expose raw exception text in API responses.
These responses still echo
${e.message}back to clients, which can leak internal DB/runtime details. Return a generic message and keep the full exception server-side.Also applies to: 179-182
🤖 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/localserver/routes/InboxRoutes.kt` around lines 114 - 129, The catch blocks in InboxRoutes.kt that handle exceptions from incomingMessagesService.select and the prior count call currently return call.respond(... "${e.message}") which leaks internal errors; change those responses to a generic message like "Failed to retrieve incoming messages" or "Failed to count incoming messages" (without interpolating e.message) and instead log the full exception server-side (e.g., logger.error or application.log.error) including the exception object; update both the select-related catch block and the earlier count-related catch block (and the same pattern at lines ~179-182) to return generic messages while logging the full e for debugging.
139-155:⚠️ Potential issue | 🟠 MajorThe inbox-by-id route is still disabled.
This handler is commented out, so the new
getById/selectByIdcode path remains unreachable and the read-by-id feature is still missing from the API surface. Either re-enable the route or drop the unused DAO/service additions from this PR.🤖 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/localserver/routes/InboxRoutes.kt` around lines 139 - 155, The inbox-by-id route block in InboxRoutes.kt (the commented get("{id}") handler that uses requireScope(AuthScopes.InboxRead), call.parameters["id"], incomingMessagesService.getById(...) and message.toDomain()) is disabled so the new read-by-id code path is unreachable; restore this API by uncommenting and re-enabling that handler (or alternatively remove the new getById/selectById service/DAO additions if you prefer to drop the feature), making sure to keep the existing null/id validation, the try/catch that maps errors to HttpStatusCode.InternalServerError, the not-found response, and the final call.respond(message.toDomain()) so the route functions correctly and enforces the AuthScopes.InboxRead check.app/src/main/assets/api/swagger.json (4)
2003-2011:⚠️ Potential issue | 🟠 MajorThe documented
202response body does not exist at runtime.
InboxRoutes.ktreturnscall.respond(HttpStatusCode.Accepted)with no payload, so this JSON object schema is a contract mismatch. Make the202response no-content.🤖 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 2003 - 2011, The OpenAPI entry for the 202 response currently defines a JSON object body but the runtime (InboxRoutes.kt where call.respond(HttpStatusCode.Accepted) is returned) sends no payload; update the swagger.json 202 response (the "202" object) to be no-content by removing the "content" / "application/json" / "schema" block (or change it to an empty response per OpenAPI) and keep a descriptive "description" indicating no body, so the contract matches the InboxRoutes.kt behavior.
514-523:⚠️ Potential issue | 🟠 MajorRemove local
type: objectfrom these enum-backed properties.Both properties already inherit
type: stringfrom the referenced enum schema viaallOf. Keepingtype: objecthere makes the schema contradictory for strict validators/codegen.Suggested fix
"type": { "allOf": [ { "$ref": "#/components/schemas/smsgateway.IncomingMessageType" } ], "description": "Message type", - "example": "SMS", - "type": "object" + "example": "SMS" } @@ "event": { "allOf": [ { "$ref": "#/components/schemas/smsgateway.WebhookEvent" } ], "description": "The type of event the webhook is triggered for.", - "example": "sms:received", - "type": "object" + "example": "sms:received" }Also applies to: 1107-1115
🤖 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 514 - 523, The property "type" currently uses allOf to reference the enum schema smsgateway.IncomingMessageType but redundantly overrides the type with "type": "object", causing a contradiction; remove the local "type": "object" entry so the property inherits the enum's "type": "string" from the referenced schema (do the same for the other occurrence at the second block around lines 1107-1115). Ensure only the allOf/$ref, description and example remain for the "type" property.
1481-1547:⚠️ Potential issue | 🟠 MajorDo not publish
/auth/token/refreshbefore the server implements it.The spec exposes this endpoint, but the provided route wiring only registers
AuthRoutes(jwtService)and this PR does not add a matching handler. Generated clients will target a dead path.🤖 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 1481 - 1547, The OpenAPI spec exposes /auth/token/refresh but there is no server handler wired up (only AuthRoutes(jwtService) is registered), so either remove the /auth/token/refresh path from swagger.json until implemented or implement and register a matching handler; to fix, either delete the "/auth/token/refresh" object from app/src/main/assets/api/swagger.json, or add a refresh token endpoint implementation (e.g., create a refreshToken method in your Auth controller/service and wire it in AuthRoutes to handle POST /auth/token/refresh, ensuring it returns the smsgateway.TokenResponse and proper error responses).
1991-1996:⚠️ Potential issue | 🟠 Major
/inbox/refreshrequest schema does not match the Kotlin handler.
InboxRoutes.ktreceivesPostMessagesInboxExportRequestwith only the period fields, but this$refpoints tosmsgateway.MessagesExportRequest, which requiresdeviceId. That produces the wrong client contract.🤖 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 1991 - 1996, The OpenAPI spec for POST /inbox/refresh incorrectly references smsgateway.MessagesExportRequest while the Kotlin handler InboxRoutes.kt expects PostMessagesInboxExportRequest (period fields only) — update the swagger.json requestBody schema reference to point to the correct component/schema (PostMessagesInboxExportRequest) or modify the smsgateway.MessagesExportRequest definition to remove deviceId and match the period-only shape used by InboxRoutes.kt so the client contract matches the handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 2003-2011: The OpenAPI entry for the 202 response currently
defines a JSON object body but the runtime (InboxRoutes.kt where
call.respond(HttpStatusCode.Accepted) is returned) sends no payload; update the
swagger.json 202 response (the "202" object) to be no-content by removing the
"content" / "application/json" / "schema" block (or change it to an empty
response per OpenAPI) and keep a descriptive "description" indicating no body,
so the contract matches the InboxRoutes.kt behavior.
- Around line 514-523: The property "type" currently uses allOf to reference the
enum schema smsgateway.IncomingMessageType but redundantly overrides the type
with "type": "object", causing a contradiction; remove the local "type":
"object" entry so the property inherits the enum's "type": "string" from the
referenced schema (do the same for the other occurrence at the second block
around lines 1107-1115). Ensure only the allOf/$ref, description and example
remain for the "type" property.
- Around line 1481-1547: The OpenAPI spec exposes /auth/token/refresh but there
is no server handler wired up (only AuthRoutes(jwtService) is registered), so
either remove the /auth/token/refresh path from swagger.json until implemented
or implement and register a matching handler; to fix, either delete the
"/auth/token/refresh" object from app/src/main/assets/api/swagger.json, or add a
refresh token endpoint implementation (e.g., create a refreshToken method in
your Auth controller/service and wire it in AuthRoutes to handle POST
/auth/token/refresh, ensuring it returns the smsgateway.TokenResponse and proper
error responses).
- Around line 1991-1996: The OpenAPI spec for POST /inbox/refresh incorrectly
references smsgateway.MessagesExportRequest while the Kotlin handler
InboxRoutes.kt expects PostMessagesInboxExportRequest (period fields only) —
update the swagger.json requestBody schema reference to point to the correct
component/schema (PostMessagesInboxExportRequest) or modify the
smsgateway.MessagesExportRequest definition to remove deviceId and match the
period-only shape used by InboxRoutes.kt so the client contract matches the
handler.
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 160-170: The request parsing try/catch in the post handler (where
call.receive<PostMessagesInboxExportRequest>().validate() is used in
InboxRoutes.kt) currently catches Exception and will swallow coroutine
cancellations; add an explicit catch for CancellationException that rethrows
(e.g., catch (e: CancellationException) { throw e }) before the broad catch (e:
Exception) so cancellations propagate correctly, and ensure
CancellationException is imported if needed.
- Around line 47-48: The handler currently uses
call.request.queryParameters["limit"]?.toIntOrNull() ?: 50 and similarly for
offset, which silently defaults when the client supplies a malformed value;
change this to first retrieve the raw param strings from
call.request.queryParameters (e.g., "limit" and "offset"), and if a param is
present but toIntOrNull() returns null respond with HTTP 400 (Bad Request) with
an explanatory message, otherwise use the parsed Int or the default (50/0).
Update the logic around the existing limit/offset variables in the Inbox route
handler to implement this validation and early response.
- Around line 114-129: The catch blocks in InboxRoutes.kt that handle exceptions
from incomingMessagesService.select and the prior count call currently return
call.respond(... "${e.message}") which leaks internal errors; change those
responses to a generic message like "Failed to retrieve incoming messages" or
"Failed to count incoming messages" (without interpolating e.message) and
instead log the full exception server-side (e.g., logger.error or
application.log.error) including the exception object; update both the
select-related catch block and the earlier count-related catch block (and the
same pattern at lines ~179-182) to return generic messages while logging the
full e for debugging.
- Around line 139-155: The inbox-by-id route block in InboxRoutes.kt (the
commented get("{id}") handler that uses requireScope(AuthScopes.InboxRead),
call.parameters["id"], incomingMessagesService.getById(...) and
message.toDomain()) is disabled so the new read-by-id code path is unreachable;
restore this API by uncommenting and re-enabling that handler (or alternatively
remove the new getById/selectById service/DAO additions if you prefer to drop
the feature), making sure to keep the existing null/id validation, the try/catch
that maps errors to HttpStatusCode.InternalServerError, the not-found response,
and the final call.respond(message.toDomain()) so the route functions correctly
and enforces the AuthScopes.InboxRead check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99375274-e20c-431b-838e-f2d5292d57e9
📒 Files selected for processing (13)
app/src/main/assets/api/swagger.jsonapp/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/EventsReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
✅ Files skipped from review due to trivial changes (2)
- app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src/main/java/me/capcom/smsgateway/modules/receiver/EventsReceiver.kt
- app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
- app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
- app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
- app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
capcom6
force-pushed
the
codex/plan-api-access-for-incoming-messages
branch
from
ae33155 to
17c95c8
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (8)
app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt (4)
187-205:⚠️ Potential issue | 🟡 MinorDTO missing
subscriptionIdfield documented in schema.The Swagger schema
smsgateway.IncomingMessageincludessubscriptionId(see lines 548-552 of swagger.json), but theInboxMessageDTO omits it. Either add the field to the DTO or remove it from the schema.🤖 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/localserver/routes/InboxRoutes.kt` around lines 187 - 205, The InboxMessage DTO is missing the subscriptionId field present in the swagger schema; update the data class InboxMessage to include a val subscriptionId: String? (or appropriate type) and update the mapping function IncomingMessage.toDomain() to set subscriptionId = subscriptionId so the domain model matches the documented smsgateway.IncomingMessage schema (refer to InboxMessage and the toDomain extension).
160-171:⚠️ Potential issue | 🟠 MajorPreserve cancellation semantics in request parsing.
The
catch (e: Exception)at line 168 will swallowCancellationExceptionfromcall.receive(...), preventing proper coroutine cancellation. Add an explicit catch forCancellationExceptionbefore the broadExceptioncatch.Suggested fix
val request = try { call.receive<PostMessagesInboxExportRequest>().validate() + } catch (e: CancellationException) { + throw e } catch (e: IllegalArgumentException) { call.respond( HttpStatusCode.BadRequest, mapOf("message" to (e.message ?: "Invalid request")) ) return@post } catch (e: Exception) {🤖 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/localserver/routes/InboxRoutes.kt` around lines 160 - 171, The current broad catch around call.receive<PostMessagesInboxExportRequest>().validate() swallows coroutine cancellations; add an explicit catch for CancellationException (from kotlinx.coroutines) before the generic Exception handler and rethrow it (or propagate it) so cancellation semantics are preserved; update the try/catch block surrounding call.receive and PostMessagesInboxExportRequest.validate to catch CancellationException first and rethrow, leaving the existing IllegalArgumentException and Exception handlers intact.
139-155:⚠️ Potential issue | 🟡 Minor
GET /{id}route is commented out (feature gap).The by-id handler is disabled, but
getById()was added to the service layer. Either enable this route or remove the service method to avoid dead code.🤖 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/localserver/routes/InboxRoutes.kt` around lines 139 - 155, The GET /{id} route handler is commented out while IncomingMessagesService.getById exists, creating dead code; either re-enable the route or delete the unused service method. To fix, restore the route block around get("{id}") in InboxRoutes.kt (including the requireScope(AuthScopes.InboxRead) check), call incomingMessagesService.getById(id) with the same error handling and map the result via toDomain() before responding, or if you prefer to remove the endpoint, delete the getById(id) implementation from IncomingMessagesService and any callers (keeping unit tests updated). Ensure references to get("{id}"), requireScope(AuthScopes.InboxRead), incomingMessagesService.getById, and toDomain() are consistently updated or removed.
114-120:⚠️ Potential issue | 🟠 MajorDo not expose raw exception messages in API responses.
Lines 117, 129, and 181 return
${e.message}directly to clients, which can leak internal details (DB/query/runtime internals). Use generic messages and log the full exception internally.Suggested fix
} catch (e: Exception) { call.respond( HttpStatusCode.InternalServerError, - mapOf("message" to "Failed to count incoming messages: ${e.message}") + mapOf("message" to "Failed to count incoming messages") )Apply the same pattern to lines 129 and 181.
🤖 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/localserver/routes/InboxRoutes.kt` around lines 114 - 120, The three catch blocks in InboxRoutes.kt that call call.respond(HttpStatusCode.InternalServerError, mapOf("message" to "Failed ...: ${e.message}")) muststop exposing raw exception text; instead log the full exception internally (e.g., logger.error("Failed to count incoming messages", e) or call.application.environment.log.error(..., e)) and return a generic error message in the response (e.g., mapOf("message" to "Failed to count incoming messages") or "Internal server error"); apply the same change to the other two catch blocks that currently interpolate ${e.message} so that all call.respond usages in those catch handlers log the exception and send only generic client-facing messages.app/src/main/assets/api/swagger.json (4)
553-562:⚠️ Potential issue | 🟠 MajorRemove contradictory
type: objecton enum-backedtypefield.The
typeproperty usesallOfto referencesmsgateway.IncomingMessageType(a string enum) while also declaringtype: objectlocally. This creates an invalid/unsatisfiable schema for strict validators. Remove the localtype: object.Suggested fix
"type": { "allOf": [ { "$ref": "#/components/schemas/smsgateway.IncomingMessageType" } ], "description": "Message type", - "example": "SMS", - "type": "object" + "example": "SMS" }🤖 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 553 - 562, The "type" property currently references the enum schema smsgateway.IncomingMessageType via allOf but also declares a conflicting local "type: object"; remove the local "type: object" declaration from the "type" property so the property solely inherits the enum type from smsgateway.IncomingMessageType (keep the description and example if desired), ensuring the property schema is consistent with the referenced string enum.
1568-1634:⚠️ Potential issue | 🟠 Major
/auth/token/refreshis documented but has no handler implementation.This endpoint is fully documented but no corresponding route handler exists in
AuthRoutes.kt. Either implement the refresh token functionality or remove this endpoint from the spec.🤖 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 1568 - 1634, The OpenAPI spec declares POST /auth/token/refresh but there is no corresponding handler in AuthRoutes.kt; either remove this path from swagger or implement the route: add a POST handler (e.g., function refreshToken or route path "/auth/token/refresh") inside AuthRoutes.kt, validate and parse the incoming refresh token, exchange it for a new access token using the existing auth service (reuse methods from login or token generation logic), return the smsgateway.TokenResponse shape on success and map failures to 401/403/500 as in other Auth routes, and register the route in the same router setup so it is reachable and covered by existing tests.
2091-2099:⚠️ Potential issue | 🟠 Major
202response documents a JSON body but handler returns none.
InboxRoutes.ktline 175 responds withcall.respond(HttpStatusCode.Accepted)(status only), but the schema declares a JSON object body. Remove the content definition to align with runtime behavior.Suggested fix
"202": { - "content": { - "application/json": { - "schema": { - "type": "object" - } - } - }, "description": "Inbox refresh request accepted" },🤖 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 2091 - 2099, The OpenAPI doc declares a JSON body for the "202" response but the handler InboxRoutes.kt uses call.respond(HttpStatusCode.Accepted) with no body; update the API spec to match runtime by removing the "content" block (application/json/schema) under the "202" response in swagger.json so the response is documented as status-only, keeping the description "Inbox refresh request accepted"; alternatively, if you prefer a body, change the handler (call.respond) to return the declared JSON object, but ensure whichever you pick keeps InboxRoutes.kt's call.respond(HttpStatusCode.Accepted) and the "202" response definition consistent.
2079-2084:⚠️ Potential issue | 🟠 Major
/inbox/refreshrequest schema mismatches handler payload.The spec references
smsgateway.MessagesExportRequestwhich requiresdeviceId, butInboxRoutes.ktreceivesPostMessagesInboxExportRequestwith onlysinceanduntil. This creates a contract/runtime mismatch. Either create a dedicated schema matchingPostMessagesInboxExportRequestor update the handler.🤖 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 2079 - 2084, The OpenAPI spec for POST /inbox/refresh incorrectly references smsgateway.MessagesExportRequest (which requires deviceId) while the Kotlin handler InboxRoutes.kt expects PostMessagesInboxExportRequest (fields: since, until); fix by either adding a new components/schemas entry (e.g., smsgateway.PostMessagesInboxExportRequest) matching the PostMessagesInboxExportRequest shape and updating the requestBody $ref to that new schema for /inbox/refresh, or modify the handler (InboxRoutes.kt) to accept and validate the existing smsgateway.MessagesExportRequest; ensure the schema fields exactly match the data class names and types used in PostMessagesInboxExportRequest so runtime payloads align.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 553-562: The "type" property currently references the enum schema
smsgateway.IncomingMessageType via allOf but also declares a conflicting local
"type: object"; remove the local "type: object" declaration from the "type"
property so the property solely inherits the enum type from
smsgateway.IncomingMessageType (keep the description and example if desired),
ensuring the property schema is consistent with the referenced string enum.
- Around line 1568-1634: The OpenAPI spec declares POST /auth/token/refresh but
there is no corresponding handler in AuthRoutes.kt; either remove this path from
swagger or implement the route: add a POST handler (e.g., function refreshToken
or route path "/auth/token/refresh") inside AuthRoutes.kt, validate and parse
the incoming refresh token, exchange it for a new access token using the
existing auth service (reuse methods from login or token generation logic),
return the smsgateway.TokenResponse shape on success and map failures to
401/403/500 as in other Auth routes, and register the route in the same router
setup so it is reachable and covered by existing tests.
- Around line 2091-2099: The OpenAPI doc declares a JSON body for the "202"
response but the handler InboxRoutes.kt uses
call.respond(HttpStatusCode.Accepted) with no body; update the API spec to match
runtime by removing the "content" block (application/json/schema) under the
"202" response in swagger.json so the response is documented as status-only,
keeping the description "Inbox refresh request accepted"; alternatively, if you
prefer a body, change the handler (call.respond) to return the declared JSON
object, but ensure whichever you pick keeps InboxRoutes.kt's
call.respond(HttpStatusCode.Accepted) and the "202" response definition
consistent.
- Around line 2079-2084: The OpenAPI spec for POST /inbox/refresh incorrectly
references smsgateway.MessagesExportRequest (which requires deviceId) while the
Kotlin handler InboxRoutes.kt expects PostMessagesInboxExportRequest (fields:
since, until); fix by either adding a new components/schemas entry (e.g.,
smsgateway.PostMessagesInboxExportRequest) matching the
PostMessagesInboxExportRequest shape and updating the requestBody $ref to that
new schema for /inbox/refresh, or modify the handler (InboxRoutes.kt) to accept
and validate the existing smsgateway.MessagesExportRequest; ensure the schema
fields exactly match the data class names and types used in
PostMessagesInboxExportRequest so runtime payloads align.
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 187-205: The InboxMessage DTO is missing the subscriptionId field
present in the swagger schema; update the data class InboxMessage to include a
val subscriptionId: String? (or appropriate type) and update the mapping
function IncomingMessage.toDomain() to set subscriptionId = subscriptionId so
the domain model matches the documented smsgateway.IncomingMessage schema (refer
to InboxMessage and the toDomain extension).
- Around line 160-171: The current broad catch around
call.receive<PostMessagesInboxExportRequest>().validate() swallows coroutine
cancellations; add an explicit catch for CancellationException (from
kotlinx.coroutines) before the generic Exception handler and rethrow it (or
propagate it) so cancellation semantics are preserved; update the try/catch
block surrounding call.receive and PostMessagesInboxExportRequest.validate to
catch CancellationException first and rethrow, leaving the existing
IllegalArgumentException and Exception handlers intact.
- Around line 139-155: The GET /{id} route handler is commented out while
IncomingMessagesService.getById exists, creating dead code; either re-enable the
route or delete the unused service method. To fix, restore the route block
around get("{id}") in InboxRoutes.kt (including the
requireScope(AuthScopes.InboxRead) check), call
incomingMessagesService.getById(id) with the same error handling and map the
result via toDomain() before responding, or if you prefer to remove the
endpoint, delete the getById(id) implementation from IncomingMessagesService and
any callers (keeping unit tests updated). Ensure references to get("{id}"),
requireScope(AuthScopes.InboxRead), incomingMessagesService.getById, and
toDomain() are consistently updated or removed.
- Around line 114-120: The three catch blocks in InboxRoutes.kt that call
call.respond(HttpStatusCode.InternalServerError, mapOf("message" to "Failed ...:
${e.message}")) muststop exposing raw exception text; instead log the full
exception internally (e.g., logger.error("Failed to count incoming messages", e)
or call.application.environment.log.error(..., e)) and return a generic error
message in the response (e.g., mapOf("message" to "Failed to count incoming
messages") or "Internal server error"); apply the same change to the other two
catch blocks that currently interpolate ${e.message} so that all call.respond
usages in those catch handlers log the exception and send only generic
client-facing messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b5cc0a9-965e-4452-80fd-0ac628174afa
📒 Files selected for processing (13)
app/src/main/assets/api/swagger.jsonapp/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.ktapp/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/EventsReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.ktapp/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
✅ Files skipped from review due to trivial changes (3)
- app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
- app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/repositories/IncomingMessagesRepository.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/main/java/me/capcom/smsgateway/modules/receiver/EventsReceiver.kt
- app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
- app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
- app/src/main/java/me/capcom/smsgateway/modules/incoming/db/IncomingMessagesDao.kt
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
1 participant
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.
Motivation
Description
smsgateway.IncomingMessageschema and new/messages/inboxand/messages/inbox/{id}endpoints toswagger.jsonto document the API.count,selectandselectByIdinIncomingMessagesDaoto support filtered counting, paginated selection and single-item lookup.count,select, andselectByIdinIncomingMessagesRepositoryand corresponding service methods inIncomingMessagesService.MessagesInboxRead, domain typesIncomingMessageResponseandGetIncomingMessagesResponse, and updatedMessagesRoutesto register inbox routes that validate parameters, enforcedeviceId, returnX-Total-Countheader, and map DB models to response DTOs.WebServiceto pass theIncomingMessagesServiceintoMessagesRoutesso the inbox routes can query stored messages.Testing
./gradlew assembleDebugto verify the code compiles successfully.Codex Task
Summary by CodeRabbit
New Features
Bug Fixes
Other