feat: local-first SMS transaction sync and storing only the necessary stuff in db - #6
Merged
Merged
Conversation
…optimistic updates, skeleton loaders, and Toast alerts
📝 WalkthroughWalkthroughThe PR adds persisted transaction drafts, backend draft endpoints, Android SMS inbox access, SMS parsing utilities, and a mobile home screen for syncing, importing, reviewing, and updating drafts. ChangesDraft SMS sync
Sequence Diagram(s)sequenceDiagram
participant HomeScreen
participant RNExpoReadSms
participant draftsRouter
participant Prisma
participant SecureStore
HomeScreen->>draftsRouter: GET /api/drafts
draftsRouter->>Prisma: list drafts for req.userId
Prisma-->>draftsRouter: drafts
draftsRouter-->>HomeScreen: { drafts }
HomeScreen->>RNExpoReadSms: readSMSInbox(limit)
RNExpoReadSms-->>HomeScreen: SMS rows
HomeScreen->>SecureStore: save last_synced_sms_date
HomeScreen->>RNExpoReadSms: startReadSMS()
RNExpoReadSms-->>HomeScreen: received_sms events
HomeScreen->>draftsRouter: POST /api/drafts or PATCH /api/drafts/:id
draftsRouter->>Prisma: upsert or update draft
Prisma-->>draftsRouter: draft
draftsRouter-->>HomeScreen: { draft }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 8
🧹 Nitpick comments (1)
backend/routes/drafts.ts (1)
117-120: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCap or paginate the drafts list.
findManyreturns every draft for a user. SMS-derived drafts can grow quickly; addtake/cursor pagination or at least a sane limit before this becomes a mobile startup hot path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/routes/drafts.ts` around lines 117 - 120, The drafts lookup in the `findMany` call is returning an unbounded list, which can become expensive for users with many drafts. Update the `drafts` query in the drafts route to apply pagination or at least a fixed `take` limit, ideally using cursor-based pagination if the endpoint needs more results. Keep the existing `where` and `orderBy` behavior in `transactionDraft.findMany`, and make sure the response shape still works with the route’s current consumers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/prisma/schema.prisma`:
- Around line 27-34: The Prisma `messageBody` field is persisting raw SMS text,
which should not be stored in the database. Update the `schema.prisma` model to
remove `messageBody` from persistence and replace it with a fingerprint or
dedupe key plus the parsed transaction fields, then adjust the `@@unique`
constraint to use the new key and existing identifiers. Also update any
draft/API flow that writes or returns `messageBody` so the full SMS body stays
local while only necessary fields are saved.
- Line 29: The amount field is using a floating-point type, which can introduce
rounding errors for money values. Update the Prisma schema’s amount field to a
decimal-backed monetary type (or store minor units), and make sure the request
parsing/handling for amount uses the corresponding decimal-safe path instead of
parseFloat. Use the amount field in the Prisma schema and the related request
parsing logic as the places to update.
- Line 31: TransactionDraft.status is still a free-form string, so invalid
values can be written through backend/routes/drafts.ts when req.body.status is
forwarded unchanged. Update the Prisma model in backend/prisma/schema.prisma to
use a dedicated enum for the draft status values (PENDING, ADDED, IGNORED), then
adjust the drafts route to validate/accept only that enum type when creating or
updating drafts so only those values can be persisted.
In `@backend/routes/drafts.ts`:
- Around line 107-109: The draft handlers in create/update/delete are leaking
internal exception details through error.message in the JSON response. Update
the catch blocks in the relevant route handlers in drafts.ts to keep logging the
full error server-side via console.error, but return a stable generic 500
response body instead of exposing Prisma/schema details; use the affected
handler functions and their shared error-response pattern to apply the same fix
consistently.
- Around line 73-103: Validate and normalize the draft payload in the drafts
route before calling prisma.transactionDraft.upsert: parse amount once and
reject null, non-numeric, or NaN values; parse date once and reject invalid Date
inputs; and restrict status to the allowed enum/values instead of accepting any
string. Update the request handling around sender, messageBody, merchant,
amount, date, and status so invalid payloads return 400, then pass only trimmed
strings and typed values into upsert/create/update.
In `@mobile/app/`(tabs)/index.tsx:
- Around line 246-247: The scan refresh logic is overwriting the entire
localDrafts state with computedDrafts, which drops unresolved
live-listener/manual-import drafts from the UI. Update the draft merge path in
the index screen logic around setLocalDrafts and syncPastTransactions so scan
results are merged into existing localDrafts instead of replacing them,
preserving any pending items that are not yet persisted. Apply the same fix in
both scan paths referenced by the comments, and make sure last_synced_sms_date
advancement does not cause older unresolved drafts to disappear.
In `@mobile/patches/`@maniac-tech__react-native-expo-read-sms.patch:
- Around line 26-44: The SMS inbox query in the read flow needs to safely handle
all cursor paths and sanitize the requested count. In the method that builds
smsList from the ContentResolver query, make sure the Cursor is closed whether
or not moveToFirst() succeeds, and do not rely on cursor.close() only inside the
populated-results branch. Also validate and clamp the limit value before passing
it into the query string so the inbox read cannot use invalid or excessively
large limits.
In `@mobile/src/lib/smsParser.ts`:
- Around line 13-16: The detector in isTransactionSms is too narrow and misses
debit alerts that parseTransactionSms can already handle, such as “debited by
250,” so align the predicate with the parser’s supported formats. Update the
amount/transaction matching logic in isTransactionSms to recognize the same
debit patterns handled by parseTransactionSms, ensuring HomeScreen’s pre-check
does not reject valid SMS messages before parsing.
---
Nitpick comments:
In `@backend/routes/drafts.ts`:
- Around line 117-120: The drafts lookup in the `findMany` call is returning an
unbounded list, which can become expensive for users with many drafts. Update
the `drafts` query in the drafts route to apply pagination or at least a fixed
`take` limit, ideally using cursor-based pagination if the endpoint needs more
results. Keep the existing `where` and `orderBy` behavior in
`transactionDraft.findMany`, and make sure the response shape still works with
the route’s current consumers.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ca9fbbf-7c16-4e89-9d07-39d942820ab2
⛔ Files ignored due to path filters (1)
mobile/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
backend/index.tsbackend/prisma/schema.prismabackend/routes/drafts.tsmobile/app.jsonmobile/app/(tabs)/index.tsxmobile/eas.jsonmobile/package.jsonmobile/patches/@maniac-tech__react-native-expo-read-sms.patchmobile/pnpm-workspace.yamlmobile/src/declarations.d.tsmobile/src/lib/smsParser.tsmobile/src/types-global.d.ts
Comment on lines
+27
to
+34
| messageBody String | ||
| merchant String | ||
| amount Float | ||
| date DateTime | ||
| status String @default("PENDING") // PENDING, ADDED, IGNORED | ||
| createdAt DateTime @default(now()) | ||
|
|
||
| @@unique([userId, sender, messageBody, date]) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Avoid persisting full SMS bodies.
messageBody stores and indexes raw SMS text, while the drafts API also writes/returns it. Bank SMS bodies can include balances, account fragments, and references; keep the full body local and persist only a fingerprint/dedupe key plus parsed fields. This also better matches the PR objective of storing only necessary DB data.
Privacy-focused schema direction
- messageBody String
+ messageHash String
+ messageSnippet String?
...
- @@unique([userId, sender, messageBody, date])
+ @@unique([userId, sender, messageHash, date])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| messageBody String | |
| merchant String | |
| amount Float | |
| date DateTime | |
| status String @default("PENDING") // PENDING, ADDED, IGNORED | |
| createdAt DateTime @default(now()) | |
| @@unique([userId, sender, messageBody, date]) | |
| messageHash String | |
| merchant String | |
| amount Float | |
| date DateTime | |
| status String `@default`("PENDING") // PENDING, ADDED, IGNORED | |
| createdAt DateTime `@default`(now()) | |
| @@unique([userId, sender, messageHash, date]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/prisma/schema.prisma` around lines 27 - 34, The Prisma `messageBody`
field is persisting raw SMS text, which should not be stored in the database.
Update the `schema.prisma` model to remove `messageBody` from persistence and
replace it with a fingerprint or dedupe key plus the parsed transaction fields,
then adjust the `@@unique` constraint to use the new key and existing
identifiers. Also update any draft/API flow that writes or returns `messageBody`
so the full SMS body stays local while only necessary fields are saved.
| sender String | ||
| messageBody String | ||
| merchant String | ||
| amount Float |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the Prisma provider and existing monetary conventions before migrating.
fd schema.prisma backend --exec sed -n '1,90p' {}
rg -n '`@db`\.Decimal|Decimal|amount' backendRepository: ankushchk/kryze
Length of output: 2283
Use a decimal-backed monetary type for amount. Float/parseFloat can introduce rounding errors for transaction amounts; switch the schema and request parsing to Decimal or minor units.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/prisma/schema.prisma` at line 29, The amount field is using a
floating-point type, which can introduce rounding errors for money values.
Update the Prisma schema’s amount field to a decimal-backed monetary type (or
store minor units), and make sure the request parsing/handling for amount uses
the corresponding decimal-safe path instead of parseFloat. Use the amount field
in the Prisma schema and the related request parsing logic as the places to
update.
| merchant String | ||
| amount Float | ||
| date DateTime | ||
| status String @default("PENDING") // PENDING, ADDED, IGNORED |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for existing Prisma enum/status conventions before applying this migration.
rg -n 'enum .*Status|status\s+String|TransactionDraftStatus' backendRepository: ankushchk/kryze
Length of output: 255
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- schema slice ---'
sed -n '1,120p' backend/prisma/schema.prisma
echo
echo '--- status usages ---'
rg -n 'TransactionDraft|\.status\b|status\s*[:=]' backend -g '!backend/prisma/schema.prisma'Repository: ankushchk/kryze
Length of output: 4625
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- draft route slice ---'
sed -n '110,180p' backend/routes/drafts.ts
echo
echo '--- status literals / draft usages ---'
rg -n 'PENDING|ADDED|IGNORED|TransactionDraft' backend -g '!backend/prisma/schema.prisma'Repository: ankushchk/kryze
Length of output: 1672
Constrain draft statuses to an enum. TransactionDraft.status is still a free-form string, and backend/routes/drafts.ts forwards req.body.status unchanged, so invalid values can be persisted. Move this to a Prisma enum so only PENDING, ADDED, and IGNORED can be stored.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/prisma/schema.prisma` at line 31, TransactionDraft.status is still a
free-form string, so invalid values can be written through
backend/routes/drafts.ts when req.body.status is forwarded unchanged. Update the
Prisma model in backend/prisma/schema.prisma to use a dedicated enum for the
draft status values (PENDING, ADDED, IGNORED), then adjust the drafts route to
validate/accept only that enum type when creating or updating drafts so only
those values can be persisted.
Comment on lines
+73
to
+103
| const { sender, messageBody, merchant, amount, date, status } = req.body; | ||
| if (!sender || !messageBody || !merchant || amount === undefined || !date || !status) { | ||
| res.status(400).json({ error: "Missing required fields" }); | ||
| return; | ||
| } | ||
|
|
||
| const userId = req.userId!; | ||
|
|
||
| const result = await prisma.transactionDraft.upsert({ | ||
| where: { | ||
| userId_sender_messageBody_date: { | ||
| userId, | ||
| sender: sender.trim(), | ||
| messageBody: messageBody.trim(), | ||
| date: new Date(date), | ||
| }, | ||
| }, | ||
| update: { | ||
| merchant: merchant.trim(), | ||
| amount: parseFloat(amount), | ||
| status, | ||
| }, | ||
| create: { | ||
| userId, | ||
| sender: sender.trim(), | ||
| messageBody: messageBody.trim(), | ||
| merchant: merchant.trim(), | ||
| amount: parseFloat(amount), | ||
| date: new Date(date), | ||
| status, | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate draft inputs before writing.
amount: null/"abc" becomes NaN, invalid dates reach Prisma as 500s, and arbitrary status values are accepted. Normalize once, reject invalid payloads with 400, then pass typed values to Prisma.
Validation direction
+const VALID_STATUSES = new Set(["PENDING", "ADDED", "IGNORED"]);
+
+function parseAmount(value: unknown): number | null {
+ const parsed = typeof value === "number" ? value : Number(value);
+ return Number.isFinite(parsed) && parsed >= 0 ? parsed : null;
+}
+
+function parseDraftDate(value: unknown): Date | null {
+ if (typeof value !== "string" && typeof value !== "number") return null;
+ const parsed = new Date(value);
+ return Number.isNaN(parsed.getTime()) ? null : parsed;
+}Also applies to: 132-151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/drafts.ts` around lines 73 - 103, Validate and normalize the
draft payload in the drafts route before calling prisma.transactionDraft.upsert:
parse amount once and reject null, non-numeric, or NaN values; parse date once
and reject invalid Date inputs; and restrict status to the allowed enum/values
instead of accepting any string. Update the request handling around sender,
messageBody, merchant, amount, date, and status so invalid payloads return 400,
then pass only trimmed strings and typed values into upsert/create/update.
Comment on lines
+26
to
+44
| + Cursor cursor = reactContext.getContentResolver().query( | ||
| + uri, | ||
| + new String[] { "address", "body", "date" }, | ||
| + null, | ||
| + null, | ||
| + "date DESC LIMIT " + limit | ||
| + ); | ||
| + | ||
| + WritableArray smsList = Arguments.createArray(); | ||
| + if (cursor != null && cursor.moveToFirst()) { | ||
| + do { | ||
| + WritableMap smsMap = Arguments.createMap(); | ||
| + smsMap.putString("sender", cursor.getString(0)); | ||
| + smsMap.putString("body", cursor.getString(1)); | ||
| + smsMap.putString("date", cursor.getString(2)); | ||
| + smsList.pushMap(smsMap); | ||
| + } while (cursor.moveToNext()); | ||
| + cursor.close(); | ||
| + } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Close the cursor on every path and bound the inbox query.
cursor.close() only runs when moveToFirst() is true, so empty inbox results leak the cursor. Also validate/clamp limit before querying to avoid invalid limits or excessive SMS reads.
Proposed fix
+ if (limit <= 0) {
+ error.invoke("limit must be greater than 0");
+ return;
+ }
+ int safeLimit = Math.min(limit, 200);
Uri uri = Uri.parse("content://sms/inbox");
- Cursor cursor = reactContext.getContentResolver().query(
+ WritableArray smsList = Arguments.createArray();
+ try (Cursor cursor = reactContext.getContentResolver().query(
uri,
new String[] { "address", "body", "date" },
null,
null,
- "date DESC LIMIT " + limit
- );
-
- WritableArray smsList = Arguments.createArray();
- if (cursor != null && cursor.moveToFirst()) {
- do {
- WritableMap smsMap = Arguments.createMap();
- smsMap.putString("sender", cursor.getString(0));
- smsMap.putString("body", cursor.getString(1));
- smsMap.putString("date", cursor.getString(2));
- smsList.pushMap(smsMap);
- } while (cursor.moveToNext());
- cursor.close();
+ "date DESC LIMIT " + safeLimit
+ )) {
+ if (cursor != null && cursor.moveToFirst()) {
+ do {
+ WritableMap smsMap = Arguments.createMap();
+ smsMap.putString("sender", cursor.getString(0));
+ smsMap.putString("body", cursor.getString(1));
+ smsMap.putString("date", cursor.getString(2));
+ smsList.pushMap(smsMap);
+ } while (cursor.moveToNext());
+ }
}
success.invoke(smsList);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| + Cursor cursor = reactContext.getContentResolver().query( | |
| + uri, | |
| + new String[] { "address", "body", "date" }, | |
| + null, | |
| + null, | |
| + "date DESC LIMIT " + limit | |
| + ); | |
| + | |
| + WritableArray smsList = Arguments.createArray(); | |
| + if (cursor != null && cursor.moveToFirst()) { | |
| + do { | |
| + WritableMap smsMap = Arguments.createMap(); | |
| + smsMap.putString("sender", cursor.getString(0)); | |
| + smsMap.putString("body", cursor.getString(1)); | |
| + smsMap.putString("date", cursor.getString(2)); | |
| + smsList.pushMap(smsMap); | |
| + } while (cursor.moveToNext()); | |
| + cursor.close(); | |
| + } | |
| if (limit <= 0) { | |
| error.invoke("limit must be greater than 0"); | |
| return; | |
| } | |
| int safeLimit = Math.min(limit, 200); | |
| Uri uri = Uri.parse("content://sms/inbox"); | |
| WritableArray smsList = Arguments.createArray(); | |
| try (Cursor cursor = reactContext.getContentResolver().query( | |
| uri, | |
| new String[] { "address", "body", "date" }, | |
| null, | |
| null, | |
| "date DESC LIMIT " + safeLimit | |
| )) { | |
| if (cursor != null && cursor.moveToFirst()) { | |
| do { | |
| WritableMap smsMap = Arguments.createMap(); | |
| smsMap.putString("sender", cursor.getString(0)); | |
| smsMap.putString("body", cursor.getString(1)); | |
| smsMap.putString("date", cursor.getString(2)); | |
| smsList.pushMap(smsMap); | |
| } while (cursor.moveToNext()); | |
| } | |
| } | |
| success.invoke(smsList); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mobile/patches/`@maniac-tech__react-native-expo-read-sms.patch around lines
26 - 44, The SMS inbox query in the read flow needs to safely handle all cursor
paths and sanitize the requested count. In the method that builds smsList from
the ContentResolver query, make sure the Cursor is closed whether or not
moveToFirst() succeeds, and do not rely on cursor.close() only inside the
populated-results branch. Also validate and clamp the limit value before passing
it into the query string so the inbox read cannot use invalid or excessively
large limits.
Comment on lines
+13
to
+16
| // Check if it looks like a transactional text containing amount and banking markers | ||
| const hasAmount = /(?:rs\.?|inr|spent)\s*[0-9,]+/i.test(lowercase); | ||
|
|
||
| return hasDebitKeywords && hasAmount; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the detector aligned with the parser.
parseTransactionSms() supports formats like debited by 250, but isTransactionSms() only accepts amounts preceded by rs|inr|spent. Since the HomeScreen checks isTransactionSms() before parsing, those debit alerts are dropped from inbox scans and manual imports entirely.
Proposed fix
- const hasAmount = /(?:rs\.?|inr|spent)\s*[0-9,]+/i.test(lowercase);
+ const hasAmount = [
+ /(?:rs\.?|inr|spent)\s*[0-9,]+(?:\.[0-9]+)?/i,
+ /debited\s*(?:by|of)?\s*(?:rs\.?|inr)?\s*[0-9,]+(?:\.[0-9]+)?/i,
+ ].some((regex) => regex.test(lowercase));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Check if it looks like a transactional text containing amount and banking markers | |
| const hasAmount = /(?:rs\.?|inr|spent)\s*[0-9,]+/i.test(lowercase); | |
| return hasDebitKeywords && hasAmount; | |
| // Check if it looks like a transactional text containing amount and banking markers | |
| const hasAmount = [ | |
| /(?:rs\.?|inr|spent)\s*[0-9,]+(?:\.[0-9]+)?/i, | |
| /debited\s*(?:by|of)?\s*(?:rs\.?|inr)?\s*[0-9,]+(?:\.[0-9]+)?/i, | |
| ].some((regex) => regex.test(lowercase)); | |
| return hasDebitKeywords && hasAmount; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mobile/src/lib/smsParser.ts` around lines 13 - 16, The detector in
isTransactionSms is too narrow and misses debit alerts that parseTransactionSms
can already handle, such as “debited by 250,” so align the predicate with the
parser’s supported formats. Update the amount/transaction matching logic in
isTransactionSms to recognize the same debit patterns handled by
parseTransactionSms, ensuring HomeScreen’s pre-check does not reject valid SMS
messages before parsing.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
backend/controllers/drafts.ts (2)
99-101: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReturn stable 500 responses instead of internal exception messages.
error.messagecan expose Prisma/schema details for attacker-controlled bad inputs. This is the same unresolved leakage issue from the earlier route implementation.Proposed fix
- res.status(500).json({ error: error.message || "Failed to create draft" }); + res.status(500).json({ error: "Failed to create draft" });- res.status(500).json({ error: error.message || "Failed to retrieve drafts" }); + console.error("Error retrieving drafts:", error); + res.status(500).json({ error: "Failed to retrieve drafts" });- res.status(500).json({ error: error.message || "Failed to update draft" }); + console.error("Error updating draft:", error); + res.status(500).json({ error: "Failed to update draft" });Also applies to: 114-115, 147-149
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/controllers/drafts.ts` around lines 99 - 101, The draft controller catch blocks are leaking internal exception details through error.message in 500 responses. Update the affected handlers in drafts.ts to return a fixed generic 500 payload instead of surfacing exception text, and keep the detailed error only in server-side logging (for example in the create/update draft route handlers and any other matching catch blocks in this file).
65-95: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate and normalize draft payloads before writing.
parseFloat()can produceNaN, invalid dates become Prisma errors, and arbitrary statuses are accepted. This is the same unresolved validation issue from the earlier route implementation, now moved into the controller.Validation direction
+const VALID_STATUSES = new Set(["PENDING", "ADDED", "IGNORED"]); + +function parseAmount(value: unknown): number | null { + const parsed = typeof value === "number" ? value : Number(value); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : null; +} + +function parseDraftDate(value: unknown): Date | null { + if (typeof value !== "string" && typeof value !== "number" && !(value instanceof Date)) return null; + const parsed = new Date(value); + return Number.isNaN(parsed.getTime()) ? null : parsed; +}Also applies to: 124-143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/controllers/drafts.ts` around lines 65 - 95, The `transactionDraft.upsert` flow in `drafts.ts` still accepts unvalidated input, so normalize and validate `req.body` before building the Prisma payload. Check that `amount` parses to a finite number, `date` becomes a valid `Date`, and `status` is one of the allowed draft statuses before calling `prisma.transactionDraft.upsert`. Reuse the existing controller path around `sender`, `messageBody`, `merchant`, `amount`, `date`, and `status` so both the create and update branches only receive sanitized values. If validation fails, return a 400 with a clear error instead of letting Prisma receive `NaN` or an invalid date.
🧹 Nitpick comments (1)
mobile/app/(tabs)/index.tsx (1)
125-145: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd accessibility labels to the icon-only buttons.
The sync, refresh, and modal-close controls have no visible text, so TalkBack/VoiceOver has no reliable accessible name here. Add explicit
accessibilityRole="button"plusaccessibilityLabel(and optionallyaccessibilityHint) on each control.Also applies to: 395-400
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mobile/app/`(tabs)/index.tsx around lines 125 - 145, The icon-only controls in the header and modal-close actions need explicit accessible names. Update the TouchableOpacity handlers in the index screen and the modal-close control referenced by the other affected block to include accessibilityRole="button" and a clear accessibilityLabel, with an accessibilityHint where useful, so TalkBack/VoiceOver can identify sync, refresh, and close actions without visible text.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/controllers/auth.ts`:
- Line 9: The JWT secret in auth.ts currently falls back to a predictable
default, which should be removed so the app fails fast when JWT_SECRET is
missing. Update the JWT_SECRET initialization in the auth controller to require
the environment variable explicitly and handle the missing-config case
immediately (for example by throwing during startup or otherwise preventing
token handling), so no code path uses a hardcoded fallback secret.
- Around line 237-263: The Twilio fallback logic in auth.ts is leaking
verification codes and can still appear to succeed when SMS is unavailable or
mocked. Update the SMS flow around twilioClient.messages.create so production
never logs the OTP or phone number in fallback paths, and do not treat the
non-Twilio branch (disableRealSms/isMockNumber) as a successful send in
production. In the catch block for smsError and the else branch after the Twilio
check, return an error response in production instead of logging the code, while
keeping any non-production diagnostics limited to safe metadata only.
- Around line 147-152: The Google sign-in flow in auth.ts currently passes
GOOGLE_CLIENT_ID directly into verifyIdToken, which can leave audience undefined
and skip token validation. Update the auth controller logic around
googleClient.verifyIdToken to fail fast when GOOGLE_CLIENT_ID is missing by
rejecting the request with an error or aborting startup before verification
runs, and keep the audience value strictly required for the token check.
- Around line 224-225: The OTP generation in auth.ts uses predictable
Math.random(), which should be replaced with cryptographic randomness. Update
the code in the OTP creation logic near normalizePhoneNumber and the code
assignment to use a secure source such as the crypto module, keeping the code
format the same while ensuring verification codes are generated unpredictably.
In `@backend/controllers/drafts.ts`:
- Around line 73-95: The upsert in draft creation is persisting the raw SMS body
via messageBody.trim(), which should be replaced with a keyed hash/fingerprint
for dedupe. Update the transactionDraft upsert in the drafts controller to use a
fingerprint field (or equivalent) in the unique key and persisted record, and
keep only parsed fields like merchant, amount, date, and status. Make sure the
create/update paths in this flow no longer store the full SMS text while
preserving the deduplication behavior.
In `@mobile/src/hooks/useHomeScreen.ts`:
- Around line 123-125: The SMS watermark is currently read from a single
device-wide SecureStore key in useHomeScreen, which causes account switches to
reuse the previous user’s sync state. Update the last_synced_sms_date storage to
be namespaced by the authenticated userId wherever it is read and written, and
make sure the sign-out flow clears or rotates that user-specific key so each
account has its own watermark.
- Around line 441-447: The amount validation in handleUpdateDraft is blocking
IGNORED drafts because parseTransactionSms() can yield 0, so update the logic to
only require a valid positive parsedAmount when status is ADDED. For the IGNORED
path, skip the positive-amount check and keep the existing draft amount
unchanged while still allowing the updateDraft flow to complete.
- Around line 350-353: The manual inbox sync in useHomeScreen is loading
lastSyncedDate but never using it, so the SMS scan keeps reprocessing
already-synced messages and regenerating local drafts. Update the sync flow
around the smsList loop to compare each item’s parsed date against the loaded
watermark and skip anything at or before lastSyncedDate, and make the same guard
in the later duplicate path referenced by the additional range so manual sync
only processes new messages.
---
Duplicate comments:
In `@backend/controllers/drafts.ts`:
- Around line 99-101: The draft controller catch blocks are leaking internal
exception details through error.message in 500 responses. Update the affected
handlers in drafts.ts to return a fixed generic 500 payload instead of surfacing
exception text, and keep the detailed error only in server-side logging (for
example in the create/update draft route handlers and any other matching catch
blocks in this file).
- Around line 65-95: The `transactionDraft.upsert` flow in `drafts.ts` still
accepts unvalidated input, so normalize and validate `req.body` before building
the Prisma payload. Check that `amount` parses to a finite number, `date`
becomes a valid `Date`, and `status` is one of the allowed draft statuses before
calling `prisma.transactionDraft.upsert`. Reuse the existing controller path
around `sender`, `messageBody`, `merchant`, `amount`, `date`, and `status` so
both the create and update branches only receive sanitized values. If validation
fails, return a 400 with a clear error instead of letting Prisma receive `NaN`
or an invalid date.
---
Nitpick comments:
In `@mobile/app/`(tabs)/index.tsx:
- Around line 125-145: The icon-only controls in the header and modal-close
actions need explicit accessible names. Update the TouchableOpacity handlers in
the index screen and the modal-close control referenced by the other affected
block to include accessibilityRole="button" and a clear accessibilityLabel, with
an accessibilityHint where useful, so TalkBack/VoiceOver can identify sync,
refresh, and close actions without visible text.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e3c21ae-52b3-4710-ad68-eb015b49890e
📒 Files selected for processing (7)
backend/controllers/auth.tsbackend/controllers/drafts.tsbackend/routes/auth.tsbackend/routes/drafts.tsmobile/app/(tabs)/index.styles.tsmobile/app/(tabs)/index.tsxmobile/src/hooks/useHomeScreen.ts
| import { prisma } from "../config/dbConnect.js"; | ||
| import { AuthRequest } from "../middleware/auth.js"; | ||
|
|
||
| const JWT_SECRET = process.env.JWT_SECRET ?? "secret-kryze-token-key-change-this-in-production"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail fast when JWT_SECRET is not configured.
A predictable fallback secret lets tokens be forged in any environment where the env var is missing.
Proposed fix
-const JWT_SECRET = process.env.JWT_SECRET ?? "secret-kryze-token-key-change-this-in-production";
+const JWT_SECRET = process.env.JWT_SECRET;
+if (!JWT_SECRET) {
+ throw new Error("JWT_SECRET must be configured");
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const JWT_SECRET = process.env.JWT_SECRET ?? "secret-kryze-token-key-change-this-in-production"; | |
| const JWT_SECRET = process.env.JWT_SECRET; | |
| if (!JWT_SECRET) { | |
| throw new Error("JWT_SECRET must be configured"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/controllers/auth.ts` at line 9, The JWT secret in auth.ts currently
falls back to a predictable default, which should be removed so the app fails
fast when JWT_SECRET is missing. Update the JWT_SECRET initialization in the
auth controller to require the environment variable explicitly and handle the
missing-config case immediately (for example by throwing during startup or
otherwise preventing token handling), so no code path uses a hardcoded fallback
secret.
Comment on lines
+147
to
+152
| const GOOGLE_CLIENT_ID = process.env.GOOGLE_CLIENT_ID; | ||
| try { | ||
| const ticket = await googleClient.verifyIdToken({ | ||
| idToken, | ||
| audience: GOOGLE_CLIENT_ID, | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
google-auth-library OAuth2Client.verifyIdToken audience option behavior when audience is undefined
💡 Result:
In the google-auth-library for Node.js, the audience option in verifyIdToken is optional [1][2]. When audience is undefined (or not provided in the VerifyIdTokenOptions object), the library does not perform an audience validation check against the ID token [3][4]. Internally, the verifyIdToken method passes the audience value directly to the underlying verifySignedJwtWithCertsAsync method [1][5]. If this value is undefined, the verification logic skips the audience matching step, meaning it will not throw an error if the aud claim in the ID token does not match a specific expected client ID [6]. While the VerifyIdTokenOptions interface marks the audience property as optional [1][7], it is considered a security best practice to provide the expected audience (your application's Client ID) to ensure that the ID token was specifically intended for your application [6]. Omitting this check could allow an ID token issued for a different client to be accepted by your backend if it is otherwise cryptographically valid [6].
Citations:
- 1: https://googleapis.dev/nodejs/google-auth-library/8.0.2/interfaces/VerifyIdTokenOptions.html
- 2: https://googleapis.dev/nodejs/google-auth-library/latest/classes/OAuth2Client.html
- 3: https://github.com/google/google-auth-library-nodejs/blob/master/src/auth/oauth2client.ts
- 4: https://googleapis.dev/nodejs/google-auth-library/8.6.0/classes/OAuth2Client.html
- 5: https://github.com/googleapis/google-auth-library-nodejs/blob/fbad5d383be691099a7dab29dca27d5263c4dfb1/src/auth/oauth2client.ts
- 6: https://stackoverflow.com/questions/54138959/google-sign-in-backend-verification
- 7: https://googleapis.dev/nodejs/google-auth-library/7.2.0/classes/OAuth2Client.html
🏁 Script executed:
sed -n '1,260p' backend/controllers/auth.ts && printf '\n---\n' && rg -n "GOOGLE_CLIENT_ID|googleClient|verifyIdToken" backend -SRepository: ankushchk/kryze
Length of output: 7775
Fail fast when GOOGLE_CLIENT_ID is missing.
verifyIdToken({ audience: undefined }) skips the audience check, so Google sign-ins can accept tokens issued for a different client. Reject the request or abort startup until this env var is set.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/controllers/auth.ts` around lines 147 - 152, The Google sign-in flow
in auth.ts currently passes GOOGLE_CLIENT_ID directly into verifyIdToken, which
can leave audience undefined and skip token validation. Update the auth
controller logic around googleClient.verifyIdToken to fail fast when
GOOGLE_CLIENT_ID is missing by rejecting the request with an error or aborting
startup before verification runs, and keep the audience value strictly required
for the token check.
Comment on lines
+224
to
+225
| const normalizedPhone = normalizePhoneNumber(phoneNumber); | ||
| const code = Math.floor(100000 + Math.random() * 900000).toString(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use cryptographic randomness for OTPs.
Math.random() is predictable and unsuitable for verification codes.
Proposed fix
+import { randomInt } from "node:crypto";- const code = Math.floor(100000 + Math.random() * 900000).toString();
+ const code = randomInt(100000, 1000000).toString();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const normalizedPhone = normalizePhoneNumber(phoneNumber); | |
| const code = Math.floor(100000 + Math.random() * 900000).toString(); | |
| import { randomInt } from "node:crypto"; | |
| const normalizedPhone = normalizePhoneNumber(phoneNumber); | |
| const code = randomInt(100000, 1000000).toString(); |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 224-224: Do not use Math.random() to generate security-sensitive values such as tokens, secrets, passwords, API keys, salts, nonces, OTPs, or session IDs. Math.random() is not cryptographically secure and is predictable. Use crypto.randomBytes()/crypto.randomUUID() (Node) or crypto.getRandomValues() (Web Crypto) instead.
Context: Math.random()
Note: [CWE-330] Use of Insufficiently Random Values.
(insecure-random-security-token-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/controllers/auth.ts` around lines 224 - 225, The OTP generation in
auth.ts uses predictable Math.random(), which should be replaced with
cryptographic randomness. Update the code in the OTP creation logic near
normalizePhoneNumber and the code assignment to use a secure source such as the
crypto module, keeping the code format the same while ensuring verification
codes are generated unpredictably.
Source: Linters/SAST tools
Comment on lines
+237
to
+263
| if (twilioClient && twilioPhoneNumber && !disableRealSms && !isMockNumber) { | ||
| try { | ||
| await twilioClient.messages.create({ | ||
| body: `Your Kryze verification code is: ${code}. It expires in 5 minutes.`, | ||
| from: twilioPhoneNumber, | ||
| to: normalizedPhone, | ||
| }); | ||
| console.log(`SMS sent successfully to ${normalizedPhone}`); | ||
| } catch (smsError: any) { | ||
| console.error("Failed to send SMS via Twilio:", smsError); | ||
| if (process.env.NODE_ENV !== "production") { | ||
| console.warn("Falling back to console logging due to Twilio error."); | ||
| console.log(`\n--- [SMS FALLBACK LOG (TWILIO ERROR)] ---`); | ||
| console.log(`To: ${normalizedPhone}`); | ||
| console.log(`Code: ${code}`); | ||
| console.log(`-----------------------------------------\n`); | ||
| } else { | ||
| res.status(500).json({ error: "Failed to send verification SMS" }); | ||
| return; | ||
| } | ||
| } | ||
| } else { | ||
| console.log(`\n--- [SMS FALLBACK LOG (SIMULATOR/MOCK)] ---`); | ||
| console.log(`To: ${normalizedPhone}`); | ||
| console.log(`Code: ${code}`); | ||
| console.log(`------------------------------------------\n`); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log OTPs or silently “send” SMS in production fallback paths.
When Twilio is disabled/misconfigured or a mock number matches, this logs the phone number and OTP and still returns success.
Proposed fix
- console.log(`SMS sent successfully to ${normalizedPhone}`);
+ console.log("SMS sent successfully"); } else {
+ if (process.env.NODE_ENV === "production") {
+ res.status(500).json({ error: "SMS delivery is not configured" });
+ return;
+ }
console.log(`\n--- [SMS FALLBACK LOG (SIMULATOR/MOCK)] ---`);
console.log(`To: ${normalizedPhone}`);
console.log(`Code: ${code}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (twilioClient && twilioPhoneNumber && !disableRealSms && !isMockNumber) { | |
| try { | |
| await twilioClient.messages.create({ | |
| body: `Your Kryze verification code is: ${code}. It expires in 5 minutes.`, | |
| from: twilioPhoneNumber, | |
| to: normalizedPhone, | |
| }); | |
| console.log(`SMS sent successfully to ${normalizedPhone}`); | |
| } catch (smsError: any) { | |
| console.error("Failed to send SMS via Twilio:", smsError); | |
| if (process.env.NODE_ENV !== "production") { | |
| console.warn("Falling back to console logging due to Twilio error."); | |
| console.log(`\n--- [SMS FALLBACK LOG (TWILIO ERROR)] ---`); | |
| console.log(`To: ${normalizedPhone}`); | |
| console.log(`Code: ${code}`); | |
| console.log(`-----------------------------------------\n`); | |
| } else { | |
| res.status(500).json({ error: "Failed to send verification SMS" }); | |
| return; | |
| } | |
| } | |
| } else { | |
| console.log(`\n--- [SMS FALLBACK LOG (SIMULATOR/MOCK)] ---`); | |
| console.log(`To: ${normalizedPhone}`); | |
| console.log(`Code: ${code}`); | |
| console.log(`------------------------------------------\n`); | |
| } | |
| if (twilioClient && twilioPhoneNumber && !disableRealSms && !isMockNumber) { | |
| try { | |
| await twilioClient.messages.create({ | |
| body: `Your Kryze verification code is: ${code}. It expires in 5 minutes.`, | |
| from: twilioPhoneNumber, | |
| to: normalizedPhone, | |
| }); | |
| console.log("SMS sent successfully"); | |
| } catch (smsError: any) { | |
| console.error("Failed to send SMS via Twilio:", smsError); | |
| if (process.env.NODE_ENV !== "production") { | |
| console.warn("Falling back to console logging due to Twilio error."); | |
| console.log(`\n--- [SMS FALLBACK LOG (TWILIO ERROR)] ---`); | |
| console.log(`To: ${normalizedPhone}`); | |
| console.log(`Code: ${code}`); | |
| console.log(`-----------------------------------------\n`); | |
| } else { | |
| res.status(500).json({ error: "Failed to send verification SMS" }); | |
| return; | |
| } | |
| } | |
| } else { | |
| if (process.env.NODE_ENV === "production") { | |
| res.status(500).json({ error: "SMS delivery is not configured" }); | |
| return; | |
| } | |
| console.log(`\n--- [SMS FALLBACK LOG (SIMULATOR/MOCK)] ---`); | |
| console.log(`To: ${normalizedPhone}`); | |
| console.log(`Code: ${code}`); | |
| console.log(`------------------------------------------\n`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/controllers/auth.ts` around lines 237 - 263, The Twilio fallback
logic in auth.ts is leaking verification codes and can still appear to succeed
when SMS is unavailable or mocked. Update the SMS flow around
twilioClient.messages.create so production never logs the OTP or phone number in
fallback paths, and do not treat the non-Twilio branch
(disableRealSms/isMockNumber) as a successful send in production. In the catch
block for smsError and the else branch after the Twilio check, return an error
response in production instead of logging the code, while keeping any
non-production diagnostics limited to safe metadata only.
Comment on lines
+73
to
+95
| const result = await prisma.transactionDraft.upsert({ | ||
| where: { | ||
| userId_sender_messageBody_date: { | ||
| userId, | ||
| sender: sender.trim(), | ||
| messageBody: messageBody.trim(), | ||
| date: new Date(date), | ||
| }, | ||
| }, | ||
| update: { | ||
| merchant: merchant.trim(), | ||
| amount: parseFloat(amount), | ||
| status, | ||
| }, | ||
| create: { | ||
| userId, | ||
| sender: sender.trim(), | ||
| messageBody: messageBody.trim(), | ||
| merchant: merchant.trim(), | ||
| amount: parseFloat(amount), | ||
| date: new Date(date), | ||
| status, | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Avoid persisting raw SMS bodies.
messageBody.trim() stores the full bank SMS, which can include balances, account references, UPI IDs, and other sensitive data. For dedupe, store a keyed hash/fingerprint instead and persist only parsed fields needed for review. This also aligns with the PR objective to store only necessary data.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/controllers/drafts.ts` around lines 73 - 95, The upsert in draft
creation is persisting the raw SMS body via messageBody.trim(), which should be
replaced with a keyed hash/fingerprint for dedupe. Update the transactionDraft
upsert in the drafts controller to use a fingerprint field (or equivalent) in
the unique key and persisted record, and keep only parsed fields like merchant,
amount, date, and status. Make sure the create/update paths in this flow no
longer store the full SMS text while preserving the deduplication behavior.
Comment on lines
+123
to
+125
| const stored = await SecureStore.getItemAsync('last_synced_sms_date'); | ||
| if (stored) { | ||
| lastSyncedDate = parseInt(stored, 10); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Scope the SMS watermark per authenticated user.
last_synced_sms_date is stored under one device-wide key, but server drafts are fetched per userId. After an account switch on the same phone, the new user inherits the previous watermark and older SMS are silently skipped. Namespace this key by the authenticated user and clear/rotate it on sign-out.
Also applies to: 336-338, 379-380
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mobile/src/hooks/useHomeScreen.ts` around lines 123 - 125, The SMS watermark
is currently read from a single device-wide SecureStore key in useHomeScreen,
which causes account switches to reuse the previous user’s sync state. Update
the last_synced_sms_date storage to be namespaced by the authenticated userId
wherever it is read and written, and make sure the sign-out flow clears or
rotates that user-specific key so each account has its own watermark.
Comment on lines
+350
to
+353
| for (const item of smsList) { | ||
| const msgDateVal = parseInt(item.date, 10); | ||
| const msgDate = !isNaN(msgDateVal) ? new Date(msgDateVal) : new Date(); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Honor lastSyncedDate during manual inbox sync.
Lines 334-342 load the watermark, but the loop never uses it. Every manual sync reparses the same inbox window and regenerates pending local drafts until each one is persisted.
Suggested fix
for (const item of smsList) {
const msgDateVal = parseInt(item.date, 10);
const msgDate = !isNaN(msgDateVal) ? new Date(msgDateVal) : new Date();
+
+ if (!isNaN(msgDateVal) && msgDateVal <= lastSyncedDate) {
+ continue;
+ }
if (isTransactionSms(item.body)) {Also applies to: 378-380
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mobile/src/hooks/useHomeScreen.ts` around lines 350 - 353, The manual inbox
sync in useHomeScreen is loading lastSyncedDate but never using it, so the SMS
scan keeps reprocessing already-synced messages and regenerating local drafts.
Update the sync flow around the smsList loop to compare each item’s parsed date
against the loaded watermark and skip anything at or before lastSyncedDate, and
make the same guard in the later duplicate path referenced by the additional
range so manual sync only processes new messages.
Comment on lines
+441
to
+447
| const handleUpdateDraft = async (status: 'ADDED' | 'IGNORED') => { | ||
| if (!selectedDraft) return; | ||
| const parsedAmount = parseFloat(editAmount); | ||
| if (isNaN(parsedAmount) || parsedAmount <= 0) { | ||
| Alert.alert('Invalid Amount', 'Please enter a valid amount'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Allow drafts to be ignored without a valid positive amount.
parseTransactionSms() falls back to 0 when it cannot extract an amount, so this guard makes those drafts impossible to mark IGNORED. Only require a positive amount for the ADDED path, or preserve the existing amount when ignoring.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mobile/src/hooks/useHomeScreen.ts` around lines 441 - 447, The amount
validation in handleUpdateDraft is blocking IGNORED drafts because
parseTransactionSms() can yield 0, so update the logic to only require a valid
positive parsedAmount when status is ADDED. For the IGNORED path, skip the
positive-amount check and keep the existing draft amount unchanged while still
allowing the updateDraft flow to complete.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit