Skip to content

Commit 89bb34c

Browse files
Programcodeclaude
andcommitted
Roll failed settings saves back to the last confirmed value
A failed save rolled back to the value on screen when it started, which could be another unconfirmed choice. The app now keeps, per setting, the value last loaded or saved successfully and rolls back to that. Saves, loads and debate writes were guarded by user ID, so work started before signing out and back in as the same account still landed. They now check an owner generation that is bumped on every owner change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent cc3a134 commit 89bb34c

2 files changed

Lines changed: 44 additions & 22 deletions

File tree

‎apps/mobile/src/lib/debate-context.tsx‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,13 @@ export function DebateProvider({ children }: { children: ReactNode }) {
3838
const [oneOff, setOneOff] = useState(false);
3939
// A load that started before a create or switch must not overwrite it, only
4040
// the latest of overlapping loads may land, and nothing from a previous
41-
// account may land after a sign-out.
41+
// owner may land after a sign-out or account switch. The generation is
42+
// bumped on every owner change, so work started before signing out and back
43+
// in as the same account does not land either.
4244
const writes = useRef(0);
4345
const refreshes = useRef(0);
4446
const owner = useRef(userId);
47+
const generation = useRef(0);
4548

4649
const refresh = useCallback(async () => {
4750
const forUser = owner.current;
@@ -64,6 +67,7 @@ export function DebateProvider({ children }: { children: ReactNode }) {
6467

6568
useEffect(() => {
6669
owner.current = userId;
70+
generation.current += 1;
6771
setDebates([]);
6872
setCurrentId(null);
6973
setOneOff(false);
@@ -86,14 +90,15 @@ export function DebateProvider({ children }: { children: ReactNode }) {
8690
oneOff,
8791
activeDebateId: oneOff ? null : current?.id ?? null,
8892
refresh,
89-
// The account is read when the student acts. If it changed while the
90-
// write ran, the write still happened for that account, so it is
91-
// reported as done but not shown to the account signed in now.
93+
// The account is read when the student acts. If the owner changed while
94+
// the write ran, the write still happened for that account, so it is
95+
// reported as done but not shown to whoever is signed in now.
9296
create: async ({ motion, settings }) => {
9397
const forUser = owner.current;
9498
if (!forUser) throw new Error('Sign in to manage debates');
99+
const gen = generation.current;
95100
const debate = await createDebate(forUser, { motion, settings });
96-
if (owner.current !== forUser) return debate;
101+
if (generation.current !== gen) return debate;
97102
writes.current += 1;
98103
setDebates((previous) => sortDebates([debate, ...previous.filter((item) => item.id !== debate.id)]));
99104
setCurrentId(debate.id);
@@ -104,8 +109,9 @@ export function DebateProvider({ children }: { children: ReactNode }) {
104109
switchTo: async (id) => {
105110
const forUser = owner.current;
106111
if (!forUser) throw new Error('Sign in to manage debates');
112+
const gen = generation.current;
107113
const at = await switchDebate(forUser, id);
108-
if (owner.current !== forUser) return;
114+
if (generation.current !== gen) return;
109115
writes.current += 1;
110116
setDebates((previous) => sortDebates(previous.map((item) => (item.id === id ? { ...item, lastActivityAt: at } : item))));
111117
setCurrentId(id);

‎apps/mobile/src/lib/settings.tsx‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -130,20 +130,27 @@ export function SettingsProvider({ children }: { children: ReactNode }) {
130130
const [settings, setSettings] = useState<Settings>(SIGNED_OUT);
131131
const [ready, setReady] = useState(false);
132132
const owner = useRef(userId);
133+
// Bumped on every owner change, including signing out and back in as the
134+
// same account. Async work from an earlier generation never lands, even when
135+
// the same account is signed in again.
136+
const generation = useRef(0);
133137
// The settings last applied, so a save builds on every earlier save.
134138
const latest = useRef(settings);
139+
// Per key, the value the account's row last held as far as the app knows:
140+
// from a load, or from a save that succeeded. A failed save rolls back to it.
141+
const confirmed = useRef<Settings>({ ...SIGNED_OUT });
135142
// Bumped by every save of a key. A load keeps the local value of any key
136-
// saved after it started, and a failed save undoes its key only while it is
137-
// still the latest save of that key.
143+
// saved after it started, and a failed save rolls its key back only while it
144+
// is still the latest save of that key.
138145
const versions = useRef<Record<SettingKey, number>>({ language: 0, outputLanguage: 0, theme: 0 });
139146
// Whether the row's feature_configs is filled in; null until a load or save tells.
140147
const configsSaved = useRef<boolean | null>(null);
141148
// Saves write one at a time in the order they were made, so the last choice
142149
// is the last write even when an earlier save's Pro-access check is slower.
143150
const [writes] = useState(createSerialQueue);
144151

145-
const apply = useCallback((next: Settings, forUser: string | null) => {
146-
if (owner.current !== forUser) return;
152+
const apply = useCallback((next: Settings, forUser: string | null, gen: number) => {
153+
if (generation.current !== gen) return;
147154
latest.current = next;
148155
setSettings(next);
149156
if (i18n.language !== next.language) void i18n.changeLanguage(next.language);
@@ -158,26 +165,32 @@ export function SettingsProvider({ children }: { children: ReactNode }) {
158165
useEffect(() => {
159166
if (loading) return;
160167
owner.current = userId;
168+
const gen = ++generation.current;
161169
configsSaved.current = null;
162170
if (!userId) {
163-
apply({ ...SIGNED_OUT, language: deviceLanguage() }, null);
171+
apply({ ...SIGNED_OUT, language: deviceLanguage() }, null, gen);
172+
confirmed.current = { ...latest.current };
164173
setReady(true);
165174
return;
166175
}
176+
confirmed.current = { ...latest.current };
167177
let cancelled = false;
168178
const started = { ...versions.current };
169-
// Loaded values, except for keys saved since the load started.
179+
// Loaded values, except for keys saved since the load started. The values
180+
// taken also become the confirmed ones.
170181
const merge = (loaded: Settings): Settings => {
171182
const next = { ...latest.current };
172183
for (const key of KEYS) {
173-
if (versions.current[key] === started[key]) Object.assign(next, { [key]: loaded[key] });
184+
if (versions.current[key] !== started[key]) continue;
185+
Object.assign(next, { [key]: loaded[key] });
186+
Object.assign(confirmed.current, { [key]: loaded[key] });
174187
}
175188
return next;
176189
};
177190
void (async () => {
178191
try {
179192
const cached = parseSettings(JSON.parse((await AsyncStorage.getItem(cacheKey(userId))) ?? 'null'));
180-
if (!cancelled && cached) apply(merge(cached), userId);
193+
if (!cancelled && cached) apply(merge(cached), userId, gen);
181194
} catch (cause) {
182195
console.warn('Could not read settings from this device:', cause);
183196
}
@@ -194,7 +207,7 @@ export function SettingsProvider({ children }: { children: ReactNode }) {
194207
}
195208
if (data && configsSaved.current === null) configsSaved.current = !isEmptyObject(data.feature_configs);
196209
const server = parseSettings(data);
197-
if (server) apply(merge(server), userId);
210+
if (server) apply(merge(server), userId, gen);
198211
})();
199212
return () => {
200213
cancelled = true;
@@ -204,19 +217,22 @@ export function SettingsProvider({ children }: { children: ReactNode }) {
204217
const save = useCallback(async <K extends SettingKey>(key: K, value: Settings[K]) => {
205218
const forUser = owner.current;
206219
if (!forUser) return;
220+
const gen = generation.current;
207221
const version = ++versions.current[key];
208-
const previous = latest.current[key];
209-
apply({ ...latest.current, [key]: value }, forUser);
222+
apply({ ...latest.current, [key]: value }, forUser, gen);
210223
try {
211224
await writes(async () => {
212-
// A save made before an account switch is dropped, not written for the new account.
213-
if (owner.current !== forUser) return;
225+
// A save made before an owner change is dropped, not written after it,
226+
// even when the same account has signed in again since.
227+
if (generation.current !== gen) return;
214228
await writeSetting(forUser, key, value, configsSaved.current !== true);
215-
if (owner.current === forUser) configsSaved.current = true;
229+
if (generation.current !== gen) return;
230+
configsSaved.current = true;
231+
Object.assign(confirmed.current, { [key]: value });
216232
});
217233
} catch (cause) {
218-
if (owner.current !== forUser) return;
219-
if (versions.current[key] === version) apply({ ...latest.current, [key]: previous }, forUser);
234+
if (generation.current !== gen) return;
235+
if (versions.current[key] === version) apply({ ...latest.current, [key]: confirmed.current[key] }, forUser, gen);
220236
throw cause;
221237
}
222238
}, [apply, writes]);

0 commit comments

Comments
 (0)