History

fix(mobile): skip unsupported shared settings targets (#9381)

Simone committed 2 months ago

Commit cfddb421 parent 57626eb6 files · +119 −69Browse files

Unified · Split

6 files
  1. apps/mobile/src/features/settings/SettingsRouteScreen.tsx +9 −12
  2. apps/web/src/hooks/useSettings.ts +15 −30
  3. docs/internals/overview.md +4 −3
  4. docs/user/thread-sidebar.md +7 −5
  5. packages/client-runtime/src/state/sharedSettings.test.ts +48 −9
  6. packages/client-runtime/src/state/sharedSettings.ts +36 −10
apps/mobile/src/features/settings/SettingsRouteScreen.tsx+9−12
@@ -46,6 +46,7 @@ import { · 45 unmodified lines
4646 import {
4747 findSharedSettingsMismatches,
4848 pickSharedServerSettings,
49+ supportsSharedSettingsSync,
4950 } from "@t3tools/client-runtime/state/shared-settings";
5051 import { useThreadListV2Enabled } from "../threads/use-thread-list-v2-enabled";
5152 import {
@@ -553,9 +554,9 @@ const AUTO_SETTLE_DEFAULT_DAYS = DEFAULT_SERVER_SETTINGS.sidebarAutoSettleAfterD · 501 unmodified lines
553554
554555 /**
555556 * Auto-settlement is a user preference that every server has to hold. Mobile
556− * has no primary environment, so the first connected environment that
557− * supports it is the reference value. Edits fan out to every connected
558− * environment, and a mismatch row lets the user push the reference out.
557+ * has no primary environment, so the first eligible sync target provides the
558+ * reference value. Edits fan out to every eligible target, and a mismatch row
559+ * lets the user push the reference out.
559560 */
560561 function AutoSettleSettingsRows() {
561562 const { environments } = useEnvironments();
@@ -564,12 +565,8 @@ function AutoSettleSettingsRows() { · 2 unmodified lines
564565 reportFailure: true,
565566 });
566567
567− const connected = environments.filter(
568− (environment) =>
569− environment.connection.phase === "connected" &&
570− environment.serverConfig?.environment.capabilities.threadAutoSettlement === true,
571− );
572− const reference = connected[0] ?? null;
568+ const syncTargets = environments.filter(supportsSharedSettingsSync);
569+ const reference = syncTargets[0] ?? null;
573570 const referenceSettings = reference?.serverConfig?.settings ?? null;
574571
575572 const [daysDraft, setDaysDraft] = useState<string | null>(null);
@@ -579,7 +576,7 @@ function AutoSettleSettingsRows() { · 3 unmodified lines
579576 }
580577
581578 const writeToAll = (patch: ServerSettingsPatch) => {
582− for (const environment of connected) {
579+ for (const environment of syncTargets) {
583580 void updateSettings({ environmentId: environment.environmentId, input: { patch } });
584581 }
585582 };
@@ -590,7 +587,7 @@ function AutoSettleSettingsRows() { · 4 unmodified lines
590587 environments: environments.map((environment) => ({
591588 environmentId: environment.environmentId,
592589 label: environment.label,
593− connected: environment.connection.phase === "connected",
590+ syncEligible: supportsSharedSettingsSync(environment),
594591 settings: environment.serverConfig?.settings ?? null,
595592 })),
596593 });
@@ -600,7 +597,7 @@ function AutoSettleSettingsRows() { · 3 unmodified lines
600597 const draft = (daysDraft ?? "").trim();
601598 setDaysDraft(null);
602599 // Whole-string check so "3.5" and "3days" are rejected instead of
603− // silently becoming 3 on every connected environment.
600+ // silently becoming 3 on every eligible sync target.
604601 const parsed = /^\d+$/.test(draft) ? Number(draft) : Number.NaN;
605602 if (
606603 Number.isInteger(parsed) &&
apps/web/src/hooks/useSettings.ts+15−30
@@ -29,6 +29,7 @@ import { · 28 unmodified lines
2929 findSharedSettingsMismatches,
3030 pickSharedServerSettings,
3131 splitSharedServerPatch,
32+ supportsSharedSettingsSync,
3233 } from "@t3tools/client-runtime/state/shared-settings";
3334 import { ensureLocalApi } from "~/localApi";
3435 import {
@@ -42,11 +43,7 @@ import * as Struct from "effect/Struct"; · 7 unmodified lines
4243 import { toastManager } from "~/components/ui/toast";
4344 import { isHostedStaticApp } from "~/hostedPairing";
4445 import { primaryServerSettingsAtom, serverEnvironment } from "~/state/server";
45−import {
46− type EnvironmentPresentation,
47− useEnvironments,
48− usePrimaryEnvironment,
49−} from "~/state/environments";
46+import { useEnvironments, usePrimaryEnvironment } from "~/state/environments";
5047 import { useAtomCommand } from "~/state/use-atom-command";
5148 import { useTheme } from "./useTheme";
5249
@@ -332,26 +329,14 @@ export function usePrimarySettingsAvailable(): boolean { · 279 unmodified lines
332329 return primaryEnvironment !== null || !isHostedStaticApp();
333330 }
334331
335−/**
336− * Whether an environment can hold every shared key right now. Gated on the
337− * auto-settlement capability because it is the newest of the shared keys: a
338− * server that has it has all of them. Older servers drop unknown keys on
339− * write, so a mismatch against them could never clear, and their decoded
340− * defaults must not be treated as real values.
341− */
342−function supportsSharedSettings(environment: EnvironmentPresentation): boolean {
343− return (
344− environment.connection.phase === "connected" &&
345− environment.serverConfig?.environment.capabilities.threadAutoSettlement === true
346− );
347−}
348−
349332 /** Environments that can receive a shared settings write right now. */
350−function useConnectedEnvironmentIds(): ReadonlyArray<EnvironmentId> {
333+function useSharedSettingsSyncTargetIds(): ReadonlyArray<EnvironmentId> {
351334 const { environments } = useEnvironments();
352335 return useMemo(
353336 () =>
354− environments.filter(supportsSharedSettings).map((environment) => environment.environmentId),
337+ environments
338+ .filter(supportsSharedSettingsSync)
339+ .map((environment) => environment.environmentId),
355340 [environments],
356341 );
357342 }
@@ -361,16 +346,16 @@ function useConnectedEnvironmentIds(): ReadonlyArray<EnvironmentId> { · 3 unmodified lines
361346 *
362347 * Server keys are optimistically patched in atom-backed server state, then
363348 * persisted via RPC. Shared server keys (see `SHARED_SERVER_SETTING_KEYS`)
364− * are written to every connected environment, not only the target, so a user
365− * preference does not silently drift between machines. Client keys go through
366− * client persistence.
349+ * are written to every eligible sync target, not only the selected target, so
350+ * a user preference does not silently drift between machines. Client keys go
351+ * through client persistence.
367352 */
368353 function useUpdateSettingsTarget(environmentId: EnvironmentId | null) {
369354 const persistServerSettings = useAtomCommand(
370355 serverEnvironment.updateSettings,
371356 "server settings update",
372357 );
373− const connectedEnvironmentIds = useConnectedEnvironmentIds();
358+ const sharedSettingsSyncTargetIds = useSharedSettingsSyncTargetIds();
374359 const updateSettings = useCallback(
375360 (patch: UnifiedSettingsPatch) => {
376361 const { serverPatch, clientPatch } = splitPatch(patch);
@@ -395,7 +380,7 @@ function useUpdateSettingsTarget(environmentId: EnvironmentId | null) { · 18 unmodified lines
395380 }
396381 }
397382 if (Object.keys(sharedPatch).length > 0) {
398− const targets = new Set(connectedEnvironmentIds);
383+ const targets = new Set(sharedSettingsSyncTargetIds);
399384 if (environmentId) {
400385 targets.add(environmentId);
401386 }
@@ -417,14 +402,14 @@ function useUpdateSettingsTarget(environmentId: EnvironmentId | null) { · 15 unmodified lines
417402 });
418403 }
419404 },
420− [connectedEnvironmentIds, environmentId, persistServerSettings],
405+ [environmentId, persistServerSettings, sharedSettingsSyncTargetIds],
421406 );
422407
423408 return updateSettings;
424409 }
425410
426411 /**
427− * Connected environments whose shared settings differ from the primary's,
412+ * Shared-settings sync targets whose values differ from the primary's,
428413 * plus an action that writes the primary's values to all of them. Drift
429414 * happens when an environment was offline during an edit or was changed by
430415 * an older client.
@@ -437,7 +422,7 @@ export function useSharedSettingsSync() { · 6 unmodified lines
437422 // must never push defaults over real values. Same for a primary too old to
438423 // hold the shared keys: its decoded defaults are not a source of truth.
439424 const primarySettings =
440− primaryEnvironment !== null && supportsSharedSettings(primaryEnvironment)
425+ primaryEnvironment !== null && supportsSharedSettingsSync(primaryEnvironment)
441426 ? (primaryEnvironment.serverConfig?.settings ?? null)
442427 : null;
443428 const { environments } = useEnvironments();
@@ -454,7 +439,7 @@ export function useSharedSettingsSync() { · 10 unmodified lines
454439 environments: environments.map((environment) => ({
455440 environmentId: environment.environmentId,
456441 label: environment.label,
457− connected: supportsSharedSettings(environment),
442+ syncEligible: supportsSharedSettingsSync(environment),
458443 settings: environment.serverConfig?.settings ?? null,
459444 })),
460445 }),
docs/internals/overview.md+4−3
@@ -90,9 +90,10 @@ A turn is complete when its session leaves `running` status, projected by · 89 unmodified lines
9090 does not define turn end.
9191
9292 Thread settlement is server-owned. Each server's own settings control PR and inactivity
93−settlement. Those keys are user preferences, so clients write them to every connected environment
94−(`SHARED_SERVER_SETTING_KEYS` in `packages/client-runtime/src/state/sharedSettings.ts`) and warn
95−when a connected environment drifts.
93+settlement. Those keys are user preferences, so clients write them to every shared-settings sync
94+target (`SHARED_SERVER_SETTING_KEYS` in `packages/client-runtime/src/state/sharedSettings.ts`) and
95+warn when another target drifts. A target must have an active connection and advertise the
96+`threadAutoSettlement` capability, which signals that the server can hold every shared key.
9697 [`ThreadSettlementReactor`][settlement] checks threads at startup, when those settings change, and
9798 once per minute, including when no client is connected. It dispatches the guarded internal
9899 `thread.auto-settle` command, which uses the existing settlement event lifecycle. Automatic
docs/user/thread-sidebar.md+7−5
@@ -22,11 +22,13 @@ available, the inactivity rule still applies. A manual un-settle also keeps the · 21 unmodified lines
2222 sorts by the moment you settled it. A thread that settled on its own sorts by its last message or
2323 turn, not by when the server noticed it was inactive.
2424
25−Change these rules in **Settings > General**. The change is written to every environment you are
26−connected to at that moment. An environment that is offline keeps its old value. When a connected
27−environment holds a different value, **Settings > General** shows a warning that names it. Choose
28−**Apply to all** to write your current values to every connected environment. The same applies to
29−the new-thread workspace mode and the source control writing style.
25+Change these rules in **Settings > General**. The change is written to every connected environment
26+whose server supports shared settings. An environment that is offline or needs a server update
27+keeps its old value and does not appear in mismatch warnings. When a connected environment whose
28+server supports shared settings holds a different value, **Settings > General** shows a warning
29+that names it. Choose **Apply to all** to write your current values to the environments named in
30+the warning. The same applies to the new-thread workspace mode and the source control writing
31+style.
3032
3133 A settings change affects future settlement and does not reopen a settled thread. Settings saved
3234 by older clients on one device no longer control this behavior.
packages/client-runtime/src/state/sharedSettings.test.ts+48−9
@@ -5,12 +5,36 @@ import { · 4 unmodified lines
55 findSharedSettingsMismatches,
66 pickSharedServerSettings,
77 splitSharedServerPatch,
8+ supportsSharedSettingsSync,
89 } from "./sharedSettings.ts";
910
1011 const primaryId = EnvironmentId.make("env-primary");
1112 const laptopId = EnvironmentId.make("env-laptop");
1213 const boxId = EnvironmentId.make("env-box");
1314
15+describe("supportsSharedSettingsSync", () => {
16+ it("accepts only connected servers that advertise the shared-settings capability", () => {
17+ expect(
18+ supportsSharedSettingsSync({
19+ connection: { phase: "connected" },
20+ serverConfig: { environment: { capabilities: { threadAutoSettlement: true } } },
21+ }),
22+ ).toBe(true);
23+ expect(
24+ supportsSharedSettingsSync({
25+ connection: { phase: "connected" },
26+ serverConfig: { environment: { capabilities: {} } },
27+ }),
28+ ).toBe(false);
29+ expect(
30+ supportsSharedSettingsSync({
31+ connection: { phase: "reconnecting" },
32+ serverConfig: { environment: { capabilities: { threadAutoSettlement: true } } },
33+ }),
34+ ).toBe(false);
35+ });
36+});
37+
1438 describe("splitSharedServerPatch", () => {
1539 it("routes preference keys to the shared patch and machine keys to the local patch", () => {
1640 const { sharedPatch, localPatch } = splitSharedServerPatch({
@@ -38,17 +62,27 @@ describe("pickSharedServerSettings", () => { · 21 unmodified lines
3862 describe("findSharedSettingsMismatches", () => {
3963 const primarySettings = { ...DEFAULT_SERVER_SETTINGS, sidebarAutoSettleAfterDays: 7 };
4064
41− it("lists connected environments whose shared settings differ", () => {
65+ it("lists sync-eligible environments whose shared settings differ", () => {
4266 const mismatches = findSharedSettingsMismatches({
4367 primaryEnvironmentId: primaryId,
4468 primarySettings,
4569 environments: [
46− { environmentId: primaryId, label: "Desktop", connected: true, settings: primarySettings },
47− { environmentId: laptopId, label: "Laptop", connected: true, settings: primarySettings },
70+ {
71+ environmentId: primaryId,
72+ label: "Desktop",
73+ syncEligible: true,
74+ settings: primarySettings,
75+ },
76+ {
77+ environmentId: laptopId,
78+ label: "Laptop",
79+ syncEligible: true,
80+ settings: primarySettings,
81+ },
4882 {
4983 environmentId: boxId,
5084 label: "Remote Box",
51− connected: true,
85+ syncEligible: true,
5286 settings: DEFAULT_SERVER_SETTINGS,
5387 },
5488 ],
@@ -64,7 +98,7 @@ describe("findSharedSettingsMismatches", () => { · 9 unmodified lines
6498 {
6599 environmentId: boxId,
66100 label: "Remote Box",
67− connected: true,
101+ syncEligible: true,
68102 settings: { ...primarySettings, enableAgentBrowserAccess: false },
69103 },
70104 ],
@@ -74,7 +108,12 @@ describe("findSharedSettingsMismatches", () => { · 3 unmodified lines
74108
75109 it("reports nothing until the primary environment's settings are loaded", () => {
76110 const environments = [
77− { environmentId: boxId, label: "Remote Box", connected: true, settings: primarySettings },
111+ {
112+ environmentId: boxId,
113+ label: "Remote Box",
114+ syncEligible: true,
115+ settings: primarySettings,
116+ },
78117 ];
79118 expect(
80119 findSharedSettingsMismatches({ primaryEnvironmentId: null, primarySettings, environments }),
@@ -88,7 +127,7 @@ describe("findSharedSettingsMismatches", () => { · 7 unmodified lines
88127 ).toEqual([]);
89128 });
90129
91− it("skips offline environments and environments without a loaded config", () => {
130+ it("skips ineligible environments and environments without a loaded config", () => {
92131 const mismatches = findSharedSettingsMismatches({
93132 primaryEnvironmentId: primaryId,
94133 primarySettings,
@@ -96,10 +135,10 @@ describe("findSharedSettingsMismatches", () => { · 1 unmodified line
96135 {
97136 environmentId: laptopId,
98137 label: "Laptop",
99− connected: false,
138+ syncEligible: false,
100139 settings: DEFAULT_SERVER_SETTINGS,
101140 },
102− { environmentId: boxId, label: "Remote Box", connected: true, settings: null },
141+ { environmentId: boxId, label: "Remote Box", syncEligible: true, settings: null },
103142 ],
104143 });
105144 expect(mismatches).toEqual([]);
packages/client-runtime/src/state/sharedSettings.ts+36−10
@@ -4,14 +4,21 @@ · 3 unmodified lines
44 * Every server keeps its own `settings.json`, but some keys are user
55 * preferences that only live on the server because the server has to act on
66 * them (auto-settlement runs with no client attached). A user does not want
7− * those to differ per machine. Clients write these keys to every connected
8− * environment, and warn when a connected environment still holds a different
9− * value so the user can push their current value out.
7+ * those to differ per machine. Clients write these keys to every shared-settings
8+ * sync target, and warn when another target still holds a different value so
9+ * the user can push their current value out.
1010 */
11−import type { EnvironmentId, ServerSettings, ServerSettingsPatch } from "@t3tools/contracts";
11+import type {
12+ EnvironmentId,
13+ ExecutionEnvironmentCapabilities,
14+ ServerSettings,
15+ ServerSettingsPatch,
16+} from "@t3tools/contracts";
1217 import * as Equal from "effect/Equal";
1318 import * as Struct from "effect/Struct";
1419
20+import type { EnvironmentConnectionPhase } from "../connection/presentation.ts";
21+
1522 /** Server keys that hold a user preference rather than machine config. */
1623 export const SHARED_SERVER_SETTING_KEYS = [
1724 "sidebarAutoSettleAfterDays",
@@ -50,18 +57,37 @@ export function pickSharedServerSettings(settings: ServerSettings): ServerSettin · 32 unmodified lines
5057 return Struct.pick(settings, SHARED_SERVER_SETTING_KEYS);
5158 }
5259
60+/**
61+ * Whether an environment can participate in shared-settings sync right now.
62+ * Auto-settlement is the newest feature backed by a shared key, so a server
63+ * advertising `threadAutoSettlement` can hold every shared key.
64+ */
65+export function supportsSharedSettingsSync(environment: {
66+ readonly connection: { readonly phase: EnvironmentConnectionPhase };
67+ readonly serverConfig: {
68+ readonly environment: {
69+ readonly capabilities: Pick<ExecutionEnvironmentCapabilities, "threadAutoSettlement">;
70+ };
71+ } | null;
72+}): boolean {
73+ return (
74+ environment.connection.phase === "connected" &&
75+ environment.serverConfig?.environment.capabilities.threadAutoSettlement === true
76+ );
77+}
78+
5379 export interface SharedSettingsEnvironment {
5480 readonly environmentId: EnvironmentId;
5581 readonly label: string;
56− readonly connected: boolean;
82+ readonly syncEligible: boolean;
5783 readonly settings: ServerSettings | null;
5884 }
5985
6086 /**
61− * Connected environments whose shared settings differ from the primary
62− * environment's. Offline environments are skipped: nothing can be read from
63− * or written to them, and the warning would never clear. With no primary
64− * settings loaded there is nothing to compare against, so nothing is
87+ * Shared-settings sync targets whose values differ from the primary
88+ * environment's. Other environments are skipped: nothing can be read from or
89+ * written to them, or their server cannot hold every shared key. With no
90+ * primary settings loaded there is nothing to compare against, so nothing is
6591 * reported. Callers must pass the real loaded settings, never a default
6692 * fallback, or "apply to all" would push defaults over real values.
6793 */
@@ -77,7 +103,7 @@ export function findSharedSettingsMismatches(input: { · 9 unmodified lines
77103 return input.environments.flatMap((environment) => {
78104 if (
79105 environment.environmentId === input.primaryEnvironmentId ||
80− !environment.connected ||
106+ !environment.syncEligible ||
81107 environment.settings === null
82108 ) {
83109 return [];