Skip to content

fix(mobile): let the signed-in session clear a start fence a previous session left behind - #513

Merged
krishagel merged 1 commit into
mainfrom
fix/unresolved-foreign-session
Sep 16, 2026
Merged

krishagel merged 1 commit into
mainfrom
fix/unresolved-foreign-session

Conversation

@krishagel

Copy link
Copy Markdown
Member

The gap #512 left

#512 made an unknown start outcome clearable — but only when the retained record matched the live owner. sameOwner compares user id, session id, and device enrollment id. Installing the new build and signing in again mints a new session id, so the record no longer matched, the snapshot became unresolved-other-session, and that screen had zero controls.

Its guidance read:

Sign back into the session that made the request to review fresh active events or contact district technology support.

A session is not re-enterable. That instruction cannot be followed. Start and join stayed blocked forever — on the very build that shipped the fix.

Observed in the field this morning on a device running the latest build.

Fix

acknowledgeUnresolved now requires only that the caller is the live signed-in owner — not that it also matches whoever wrote the record. Whoever holds the device and is signed in can clear it.

That is safe because the record was never verifiable by its original session either: it carries no proof of what the server did, and the active-event list it would be checked against carries no request-specific idempotency correlation (which is exactly why resolveActivationFromFreshEvents was removed). Retaining it against an owner who can never return only disabled the device.

OtherSessionStartMutationAttention gains the same explicit control the owner-visible screen already has, and honest guidance in place of the impossible instruction. It still shows no classified event details from the other session — this is a clear, not a disclosure — and the control is disabled while offline.

Coverage

  • A record written by one session is cleared by the next sign-in on the same device, and the following start is admitted.
  • A signed-out device refuses the clear and keeps the record.
  • The previous-session screen exposes the control, fires it once, no longer contains the impossible instruction, and still leaks no event identity.
  • The control is disabled offline.

Restoring the old owner-match guard makes the first test fail, which is the field case reproduced.

Gate

Prettier, ESLint, typecheck clean repo-wide. 6/6 test shards green; 61 mobile native tests pass. (One earlier shard failure was load flake while EAS builds were running — 581s vs 59s on a quiet machine; clean on re-run.)

Platform parity

Client-only logic and shared React Native UI — no platform-specific code. Needs verifying on iOS and Android.

… session left behind

A device that had latched an unknown start outcome could still be unusable
after installing the build that was supposed to fix it. Updating the app and
signing in again mints a new session id; `sameOwner` compares user, session
and device enrollment, so the retained record no longer matched the live
owner, the snapshot became `unresolved-other-session`, and that screen carried
no controls of any kind. Its guidance told the operator to "sign back into the
session that made the request", which is not a thing a person can do: a
session is not re-enterable. Start and join stayed blocked with no exit, on a
build that already shipped the exit for the same-session case.

`acknowledgeUnresolved` now requires only that the caller is the live
signed-in owner, not that it also matches whoever wrote the record. Whoever
holds the device and is signed in may clear it. Nothing about the record was
ever verifiable by the original session anyway: it carries no proof of what
the server did, and the active-event list it would have been checked against
carries no request-specific idempotency correlation. Retaining it against an
owner who can never return only disabled the device.

`OtherSessionStartMutationAttention` gains the same explicit control the
owner-visible screen has, and its guidance now says the earlier session has
ended and cannot be signed back into rather than asking for the impossible.
The screen still shows no classified event details from the other session, so
the control reveals nothing: it is a clear, not a disclosure. The clear is
disabled while offline, matching the owner-visible screen.

Coverage: a record written by one session is cleared by the next sign-in on
the same device and the following start is admitted; a signed-out device
refuses the clear and keeps the record; the previous-session screen exposes
the control, fires it once, no longer contains the impossible instruction, and
still leaks no event identity; the control is disabled offline.

Gate: Prettier, ESLint and typecheck clean across the repository; 6/6 test
shards green and 61 mobile native tests pass.
Copilot AI lite review requested due to automatic review settings September 16, 2026 15:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@claude

claude Bot commented Sep 16, 2026

Copy link
Copy Markdown

Review: PR #513 — let the signed-in session clear an orphaned start fence

Summary: clean diff. No IMPORTANT findings.

What it does
acknowledgeUnresolved in start-mutation-coordinator.ts:882-901 drops the sameOwner(this.state.owner, owner) check and now only requires sameOwner(this.onlineOwner, owner). This fixes a real bug: a new sign-in mints a new sessionId, so sameOwner (which compares userId + sessionId + deviceEnrollmentId) could never match again, permanently blocking start/join on the device (start-mutation-coordinator.ts:315-326).

Correctness

  • The removed check has no effect on the same-session path (if you're already the record's owner, both checks always agreed), so this is scoped precisely to the 'other session' branch — verified against visibleSnapshot() at start-mutation-coordinator.ts:524-527.
  • acknowledge() (succeeded/failed terminal states, start-mutation-coordinator.ts:854-867) and claimSuccessFeedback (:939-958) still require exact owner match — correctly left alone, since only the unresolved-record case has an unreachable original owner.
  • New tests (start-mutation-coordinator.test.ts:1029-1073) exercise both the fixed path (next sign-in clears the fence, and can then submit a new activation) and the guard (acknowledgeUnresolved still refused when nobody is signed in). Both assert on getSnapshot().phase / persistence.record, not just return values — real behavioral assertions.
  • Component tests (start-mutation-attention.test.tsx:623-673) assert the impossible 'sign back into the session' copy is gone, the new copy is present, the clear control fires the callback, and — importantly — that no other session's classified event details ('Synthetic earthquake', 'REAL INCIDENT') leak into this screen. Also covers the offline-disabled case.
  • packages/mobile/src/app/index.tsx and .../app/start/index.tsx both wire online/onAcknowledgeUnresolved consistently; the start/index.tsx variant additionally calls returnHome() since it's reached from the start flow rather than the home screen — appropriate, not an inconsistency.

Security / data handling

  • This only clears local on-device coordinator state (persistence.clear()), never contacts a server, and per the design (and tests) never renders the other session's event/classification details on this screen. No FERPA/PII exposure, no new logging or external calls.
  • Worth naming explicitly (not a blocker, matches the PR body's own reasoning): the new check authorizes any currently-signed-in owner on the device to clear a fence left by a different prior owner — it no longer checks that the clearing user is even the same person, only that they're currently authenticated on the device. The PR body acknowledges this tradeoff directly and the local-only blast radius (no destroyed server truth, no disclosed data) makes it reasonable, but flagging it since it's a real widening of who can dismiss the record versus the pre-PR intent of 'same person, new session.'

Nits (2)

  • start-mutation-coordinator.ts:884-890: the comment is long (7 lines) for a one-line behavior change; could trim now that the JSDoc above acknowledgeUnresolved (:869-880) already explains most of the rationale.
  • start-mutation-attention.tsx:670-672: neutralAcknowledgeAction duplicates the #6B3A05 literal already used for the ActivityIndicator color a few lines up — minor, pre-existing pattern in this file.

No CI weakening, no migrations, no .database.test.ts touched.

@krishagel
krishagel merged commit edab07e into main Sep 16, 2026
12 checks passed
@krishagel
krishagel deleted the fix/unresolved-foreign-session branch September 17, 2026 02:37
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