Skip to content

Added hold to chat feature for SOS avoid manual typing, One tap to send any signal to your loved ones, worked on UI and UX - #67

Open
gauravk16in wants to merge 3 commits into
ni5arga:mainfrom
gauravk16in:main
Open

Added hold to chat feature for SOS avoid manual typing, One tap to send any signal to your loved ones, worked on UI and UX #67
gauravk16in wants to merge 3 commits into
ni5arga:mainfrom
gauravk16in:main

Conversation

@gauravk16in

Copy link
Copy Markdown

Hold and Chat: Added support for hold-and-chat interactions.
Quick Replies: Added 5 predefined messages to avoid manual typing during critical situations: "I'm safe", "Medical", "Need help", "Unsafe situation", and "Lost my group".
Enhanced UI and UX: Upgraded the overall user interface and experience for faster, more intuitive access during emergencies.

Kr added 3 commits July 22, 2026 02:13
Extends MessageBody with two new sealed-payload variants:
- emergency: category, urgency, location (text label, no GPS), id, sentAt
- heartbeat: state:'safe', id, sentAt

Also adds:
- EmergencyCategory const object (closed set of 4 valid values)
- EmergencyLocationLabel type (text label or none — GPS absent by design)
- EMERGENCY_TTL_SECONDS = 15 min (vs 6h for chat messages)
- parseLocation() with 200-char label clamp and whitespace trimming
- 20 new protocol tests covering all rejection cases

No changes to: mesh engine, transport, crypto, storage, or any UI.
Old peers receive new message kinds as null (silent discard) — backward compatible.

Reviewed against:
- privacy: no location by default, sentAt is minute-rounded
- security: field-by-field rebuild, extra keys stripped
- scale: JSON encoding <100 bytes, fits existing 256-byte bucket
- backward compat: unknown kind → null → dropped by old app

@commitchan commitchan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gauravk16in — thanks for putting real work into this. The underlying instinct is correct and it is something the app genuinely lacks: under stress, typing is the wrong interface, and one-tap "I'm safe" / "Medical" is the right shape for a phone held in a crowd. The hold-to-confirm gesture is also the right anti-misfire pattern for a destructive-if-wrong action. I want the feature.

I cannot merge this version, and it is not close. Below is everything I found, verified locally on a merge of this branch onto a0e6dc9.

It does not build

No CI ran on this PR at all — the head is your fork's main, and the checks never triggered, so nothing here was ever verified. Running the repo's own gates:

npm run typecheck  → 5 errors
npm test           → 1 failure (266/267)
npm run lint       → 7 errors

All five typecheck errors are the same cause: 30 new emergency.* keys were added to src/i18n/en.ts only, so bn/hi/mr/ta/te no longer satisfy Catalog. The test failure is translation catalogs → contain exactly the English keys. Note that npm run i18n:check passes — it audits different properties, so it is not a substitute for the other two.

The lint errors are real React bugs, not style: react-hooks/refs "Cannot access refs during render" at src/app/emergency/sos.tsx:141,145,261, and "Cannot call impure function during render" at src/components/alert-feed.tsx:52. Under the new React compiler semantics those are the kind of thing that works in dev and misbehaves in a release build — on the screen someone opens when they are in trouble.

Beyond the tooling: this app ships in six languages specifically for the context it was built for. An English-only emergency UI means the users least able to read it are the ones the feature exists for.

Design problems that need resolving before code

These are not "fix the lint" items. They are the reason I want a design conversation on the issue before another push.

1. The report count is attacker-controlled, and the UI presents it as social proof

emergency-protocol.ts states: "The receiver-side deduplication (window_bucket UNIQUE index in db.ts) stops intentional spam from any single source." That is not what the index does. I ran your schema and upsert against SQLite directly:

different id, same (sender, category, bucket)  → OK, report_count = 2
same id,      same (sender, category, bucket)  → OK, report_count = 3
same id,      different bucket                 → THREW: UNIQUE constraint failed: emergency_alerts.id

The first line is the problem. The unique index does not suppress repeats from one source — it merges them into a single row with a larger number. id comes off the wire from the sender, so one device sending N alerts in one 5-minute window produces report_count = N, which alert-feed.tsx renders through emergency.reportCount.other as "N people nearby". The cooldown in COOLDOWN_MS is client-side, in-memory, and enforced by the sending device on itself — it is not a defence against anyone who does not want to be limited.

Your own setupfile.md Test C documents this and treats it as correct: restart the app to clear the limiter, send again, "the alert feed should now say 2 people nearby". One phone. That is the bug, written down as the acceptance criterion.

emergency.trustNote ("Reported by nearby user — not verified") is a good instinct but it cannot carry this. Under stress a number wins over a caveat, and "7 people nearby" is precisely the input someone uses to decide whether to run toward or away from something. A count that one device can set to any value is worse than no count.

The third line is a smaller robustness issue: the upsert's conflict target is the dedup index, so a collision on the id primary key is not handled and the insert throws. It is caught at app-state.tsx and downgraded to a console.warn, so it is a silent drop rather than a crash — but it is a silent drop on an emergency path.

2. Pressing SOS broadcasts your long-term identity in the clear

sealToKey places sender.edPublic and sender.xPublic inside the sealed inner (crypto-core.ts:683), and handleIncomingEmergencyBytes stores senderPublicId as sender_id. EMERGENCY_CHANNEL_KEY is hkdf(sha256, 'protestchat/v1/emergency-broadcast', …) — a constant anyone can compute from this repository without installing anything.

So the actual effect of tapping "Medical" is: broadcast my permanent public identity, my emergency category, and optionally my area label, to every device in Bluetooth range, in a form anyone can read. That is the same identity that is on my QR contact code. Anyone who has ever scanned me, or collected codes at previous events, can now place me at a location, at a time, in distress.

This is the opposite of what the rest of the app spends its effort on — the rotating BLE advertisement tags exist precisely so there is no stable identifier on the air. emergency.sos.trustWarning warns about reach ("will reach all phones running this app within Bluetooth range. There is no way to recall it") but says nothing about identity, so a user cannot consent to the trade they are actually making.

This is solvable — an ephemeral per-alert key, or a mode that carries no author at all — but it needs deciding before implementing, because it changes the wire format.

Related, and worth stating plainly: the same constant lets anyone forge alerts. The module acknowledges "Neither prevents a hostile device from flooding from many different identities — that is an accepted limitation." Accepting that is defensible for a decentralised system. What is not defensible is accepting it and rendering an unauthenticated count as a confidence signal. Pick one.

3. The 15-minute TTL is documented but never applied

EMERGENCY_TTL_SECONDS = 15 * 60 is defined in protocol.ts:428, imported by emergency-protocol.ts:46, re-exported at :269, and used nowhere. sendEncodedToKey calls inject(sealed), and inject hardcodes ttlSeconds: DEFAULTS.ttlSeconds — 6 hours (mesh.ts:484).

So the doc comment's claims are false on the wire. Emergency envelopes propagate and are retained for six hours, not fifteen minutes. The 15-minute sweep only clears the local emergency_alerts/heartbeats tables; the sealed envelope stays in every relay's envelopes table for the full 6h — and because the key is global, every one of those relays can decrypt it. A seized relay phone therefore contains six hours of readable emergency alerts, complete with sender identities, which is precisely the outcome the comment says the design prevents.

That is the one that worries me most, because the comment is confident and the code does not do it. On this app the dangerous failure is not a crypto break — it is a user who believed a sentence we wrote.

Scope and hygiene

This is 2,077 lines across 20 files spanning crypto, protocol, mesh, db, state, UI, docs, and a new dependency. Per CONTRIBUTING (landed in #63): keep the diff reviewable and separate unrelated changes. Concretely:

  • setupfile.md at the repo root is a personal testing scratchpad and should not land. If the manual test plan is worth keeping, it belongs in the PR description or under docs/.
  • docs/banner.jpg is replaced with different binary content and no explanation.
  • PROPOSAL.md picks up a stray backtick on line 70, breaking the rendered document.
  • src/lib/mesh.ts:13 — a module doc comment now reads "costs bandwidth and battery a nd". Small, but it means the file was edited without being read back.
  • The README section describes the change in changelog voice ("Added support for…", "Enhanced UI and UX"). The README documents what the app is, not what a PR did.
  • expo-linear-gradient is a new runtime dependency for visual styling. Worth its own justification given this ships as an APK to people who may be sideloading it.

What I would like to happen

Please do not force-push a fixed version of this diff. Open an issue for the emergency-coordination design and let us settle three things first: whether an alert carries an author at all, what number (if any) the UI is allowed to show given it cannot be authenticated, and what the real wire TTL should be. @ni5arga should weigh in — this adds a new always-on broadcast channel to the threat model, which is a maintainer call, not a review call.

Once that is agreed, split it: protocol + crypto + db in one PR with tests, UI in another. That is reviewable, and I will turn both around quickly.

The idea is good and I would rather see it land properly than not at all.

@gauravk16in

Copy link
Copy Markdown
Author

Thank you so much for taking the time to review this in such depth. I genuinely appreciate the detailed feedback.
You're right that I bundled too many changes into one PR. Looking back, I should have split the protocol, backend, and UI changes into separate PRs as suggested.
My goal was to contribute something that could be useful quickly, but I understand that for a project like this, correctness, privacy, and security matter more than speed. I don't want to compromise those principles.
Regarding the localization issues, I currently only implemented the English strings because I wasn't familiar with the project's translation workflow. I'll spend some time understanding how the catalogs are maintained before submitting another PR.
I also only had access to Android devices for testing, so I verified the feature on two physical Android phones. I saw the same failing tests you mentioned, but I now understand that local testing isn't enough, and I shouldn't have opened the PR until the project's quality gates were passing.
I'll open a design discussion for the emergency feature as suggested.
Once again thank you for giving time to my pR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants