Repository navigation
Warn before leaving until report is saved - #250
davidtsai15 wants to merge 2 commits into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply to a comment to ask cubic a question or push back. It learns from your replies.
Turn on auto-fix | Re-trigger cubic
|
|
||
| function finalizeRecoveryOnPageHide() { | ||
| if (state.phase === "report_recovery") reportRecovery.finish(); | ||
| void state.room?.disconnect?.().catch?.(() => {}); |
There was a problem hiding this comment.
Since pagehide now owns the room disconnect, leaveRoom tears down too early. It still calls state.room?.disconnect(), stopAvatar() and stopLocalMedia() before setting window.location.href = "/". In phase ending with no saved report, the guard is still armed, so that assignment raises the native prompt. A candidate who picks Stay is left on the ending screen with the room gone, so the report they are waiting for can never arrive. Either release the guard in leaveRoom before navigating (the button is already an explicit choice to leave), or drop the disconnect and media teardown there and let this handler do it. A test that cancels navigation through the real Leave button would catch this.
There was a problem hiding this comment.
Released the unload guard before an explicit Leave action, as suggested, so the existing teardown can run without triggering the accidental-navigation prompt. Added a regression test covering the Leave-button path.
| state.reportPersisted = false; | ||
| updateBeforeUnloadGuard(); | ||
| const result = await saveReportHistory(entry); | ||
| if (reportIsPersisted(result)) { |
There was a problem hiding this comment.
An older save that finishes late can release the guard while the newer copy is still unsaved. In report recovery, keep() saves the provisional report, then finalize saves the outcome under the same id. If the provisional account request is still in flight when the final save sets state.reportPersisted = false, its later success sets it back to true. If both writes of the final copy then fail, the page is unguarded and the final evaluation is lost without a warning. That contradicts the comment above it, which says only this save can release the warning. Tag each call with a generation number and let only the latest one write the flag:
| if (reportIsPersisted(result)) { | |
| if (generation === saveGeneration && reportIsPersisted(result)) { |
with const generation = ++saveGeneration; next to the reset, plus a test with overlapping provisional and final saves.
There was a problem hiding this comment.
Added a save generation counter so only the newest report save can release the unload guard. Also added an overlapping provisional/final save regression test where the older save finishes after the newer save has started.
A timed interview can be lost to refresh or navigation before its report reaches Past attempts. LiveKit's page-leave hook also disconnects before the candidate can choose Stay, so wait for pagehide and keep the warning until a saved report can be reopened.
Let an explicit Leave bypass the accidental-navigation warning. Ignore stale save completions after a newer report save starts.
a14db9c to
aef6158
Compare
2 participants
Timed interviews currently allow accidental refreshes or navigation before the report is safely persisted.
This change keeps the native beforeunload guard active from the timed interview start until either the local or account report copy has been saved. LiveKit's automatic page-leave disconnect is disabled because it runs before the candidate chooses whether to stay, so room cleanup now waits for pagehide.
Tests cover the interview lifecycle, report persistence results, listener cleanup, and LiveKit page-leave behavior. The credential-free test gate passes locally, including all 936 browser tests with no failures or skips.
Closes #243
Summary by cubic
Warns before leaving a timed interview until a report copy is saved, so candidates can't lose an unsaved report by refreshing or navigating away.
pagehideso it doesn't run before the candidate chooses to stay.Closes #243.
Written for commit aef6158. Summary will update on new commits.