From 48bc314dfc236291cf4fc4cf8d87abc663985007 Mon Sep 17 00:00:00 2001 From: engineer Date: Wed, 3 Jun 2026 14:01:29 -0700 Subject: [PATCH] =?UTF-8?q?test(stores):=20cover=20settings=20merge/clamp?= =?UTF-8?q?=20+=20notification=20sanitizer=20(29=E2=86=9242=20tests)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract two pure helpers so they're testable without zustand/expo: - settings-merge.ts: clampPageSize + forward-compatible mergeStoredSettings (the upgrade path where stored data predates a new notification category must yield the default, not undefined). stores/settings.ts now delegates. - notify-format.ts: sanitizeBody (strip C0/DEL control chars, trim, cap at 200) used for server-supplied notification text. stores/events.ts now imports it. Behavior unchanged; typecheck clean. Co-Authored-By: Claude Opus 4.8 --- src/lib/notify-format.test.ts | 41 +++++++++++++++++++++++++++ src/lib/notify-format.ts | 14 +++++++++ src/lib/settings-merge.test.ts | 52 ++++++++++++++++++++++++++++++++++ src/lib/settings-merge.ts | 25 ++++++++++++++++ src/stores/events.ts | 5 +--- src/stores/settings.ts | 8 +++--- 6 files changed, 137 insertions(+), 8 deletions(-) create mode 100644 src/lib/notify-format.test.ts create mode 100644 src/lib/notify-format.ts create mode 100644 src/lib/settings-merge.test.ts create mode 100644 src/lib/settings-merge.ts diff --git a/src/lib/notify-format.test.ts b/src/lib/notify-format.test.ts new file mode 100644 index 0000000..65da5c2 --- /dev/null +++ b/src/lib/notify-format.test.ts @@ -0,0 +1,41 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { sanitizeBody, MAX_NOTIF_BODY } from "./notify-format.ts" + +// Notification bodies come from the server (and ultimately the AI). They must be +// neutralized before display: control chars stripped, whitespace trimmed, length capped. + +test("returns the fallback for undefined or empty input", () => { + assert.equal(sanitizeBody(undefined, "fb"), "fb") + assert.equal(sanitizeBody("", "fb"), "fb") +}) + +test("returns the fallback when input is only whitespace/control chars", () => { + assert.equal(sanitizeBody(" ", "fb"), "fb") + assert.equal(sanitizeBody("\x00\x07\x1f", "fb"), "fb") +}) + +test("passes ordinary text through, trimmed", () => { + assert.equal(sanitizeBody(" hello world ", "fb"), "hello world") +}) + +test("replaces C0 control chars and DEL with spaces", () => { + // newline, tab, bell, DEL between the words become spaces + assert.equal(sanitizeBody("a\nb\tc\x07d\x7fe", "fb"), "a b c d e") +}) + +test("does not strip non-ASCII/emoji content", () => { + assert.equal(sanitizeBody("café 🚀 プロジェクト", "fb"), "café 🚀 プロジェクト") +}) + +test("caps length at MAX_NOTIF_BODY", () => { + const long = "x".repeat(500) + const out = sanitizeBody(long, "fb") + assert.equal(out.length, MAX_NOTIF_BODY) + assert.equal(out, "x".repeat(MAX_NOTIF_BODY)) +}) + +test("trims before slicing so leading whitespace doesn't eat the budget", () => { + const out = sanitizeBody(" " + "y".repeat(250), "fb") + assert.equal(out, "y".repeat(MAX_NOTIF_BODY)) +}) diff --git a/src/lib/notify-format.ts b/src/lib/notify-format.ts new file mode 100644 index 0000000..5a1e149 --- /dev/null +++ b/src/lib/notify-format.ts @@ -0,0 +1,14 @@ +// Pure formatting for notification bodies, extracted from stores/events.ts so the +// sanitization rules are unit-testable without the event store's RN dependencies. + +export const MAX_NOTIF_BODY = 200 + +/** + * Make a server-supplied string safe to show in a notification: + * strip C0 control characters and DEL (which can corrupt the notification shade + * or hide content), collapse surrounding whitespace, and cap the length. Falls + * back to `fallback` when the input is empty/whitespace-only or undefined. + */ +export function sanitizeBody(s: string | undefined, fallback: string): string { + return (s ? s.replace(/[\x00-\x1f\x7f]/g, " ").trim().slice(0, MAX_NOTIF_BODY) : "") || fallback +} diff --git a/src/lib/settings-merge.test.ts b/src/lib/settings-merge.test.ts new file mode 100644 index 0000000..20ea673 --- /dev/null +++ b/src/lib/settings-merge.test.ts @@ -0,0 +1,52 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { clampPageSize, mergeStoredSettings } from "./settings-merge.ts" + +const DEFAULTS = { + pageSize: 25, + notifications: { idle: true, error: true, permission: false }, +} + +test("clampPageSize keeps in-range values unchanged", () => { + assert.equal(clampPageSize(25), 25) + assert.equal(clampPageSize(10), 10) + assert.equal(clampPageSize(200), 200) +}) + +test("clampPageSize floors below 10 and caps above 200", () => { + assert.equal(clampPageSize(0), 10) + assert.equal(clampPageSize(-5), 10) + assert.equal(clampPageSize(9), 10) + assert.equal(clampPageSize(201), 200) + assert.equal(clampPageSize(99999), 200) +}) + +test("empty stored settings yield the defaults", () => { + assert.deepEqual(mergeStoredSettings(DEFAULTS, {}), DEFAULTS) +}) + +test("stored values override defaults", () => { + const merged = mergeStoredSettings(DEFAULTS, { pageSize: 50 }) + assert.equal(merged.pageSize, 50) + assert.deepEqual(merged.notifications, DEFAULTS.notifications) +}) + +test("upgrade path: a category missing from storage gets its default", () => { + // Stored data predates the "permission" category — it must come back as the default (false), + // not undefined, while the user's stored choices are preserved. + const merged = mergeStoredSettings(DEFAULTS, { notifications: { idle: false, error: true } }) + assert.equal(merged.notifications.idle, false) // user's stored choice kept + assert.equal(merged.notifications.error, true) + assert.equal(merged.notifications.permission, false) // new category -> default + assert.equal("permission" in merged.notifications, true) +}) + +test("does not mutate the inputs", () => { + const defaults = { pageSize: 25, notifications: { a: true } } + const parsed = { notifications: { a: false } } + const merged = mergeStoredSettings(defaults, parsed) + assert.equal(defaults.notifications.a, true) // untouched + assert.equal(parsed.notifications.a, false) // untouched + assert.equal(merged.notifications.a, false) + assert.notEqual(merged.notifications, defaults.notifications) +}) diff --git a/src/lib/settings-merge.ts b/src/lib/settings-merge.ts new file mode 100644 index 0000000..f1175f6 --- /dev/null +++ b/src/lib/settings-merge.ts @@ -0,0 +1,25 @@ +// Pure settings helpers extracted from stores/settings.ts so the clamp + the +// forward-compatible merge (the upgrade path: stored data from an older app +// version that predates a newer notification category) are unit-testable without +// pulling in zustand / expo-secure-store. + +export function clampPageSize(size: number): number { + return Math.max(10, Math.min(200, size)) +} + +/** + * Merge stored settings over defaults. Stored values win, but any top-level field + * or notification category missing from storage falls back to its default — so a + * user upgrading to a build with a new setting gets that setting's default rather + * than `undefined`. Defaults are passed in to keep this module dependency-free. + */ +export function mergeStoredSettings }>( + defaults: T, + parsed: Partial, +): T { + return { + ...defaults, + ...parsed, + notifications: { ...defaults.notifications, ...parsed.notifications }, + } +} diff --git a/src/stores/events.ts b/src/stores/events.ts index 75a003c..044ab86 100644 --- a/src/stores/events.ts +++ b/src/stores/events.ts @@ -2,10 +2,7 @@ import { create } from "zustand" import { useConnections } from "./connections" import { useSessions } from "./sessions" import { send as notify } from "../lib/notifications" - -const MAX_NOTIF_BODY = 200 -const sanitizeBody = (s: string | undefined, fallback: string): string => - (s ? s.replace(/[\x00-\x1f\x7f]/g, " ").trim().slice(0, MAX_NOTIF_BODY) : "") || fallback +import { sanitizeBody } from "../lib/notify-format" import { addBreadcrumb } from "../lib/sentry" import type { Client, Part, Session, Message } from "../lib/sdk" diff --git a/src/stores/settings.ts b/src/stores/settings.ts index b3bc3ed..5ab86d9 100644 --- a/src/stores/settings.ts +++ b/src/stores/settings.ts @@ -1,6 +1,7 @@ import { create } from "zustand" import * as SecureStore from "expo-secure-store" import { type Category, defaultPreferences } from "../lib/notifications" +import { clampPageSize, mergeStoredSettings } from "../lib/settings-merge" const SETTINGS_KEY = "opencode_settings" @@ -37,16 +38,15 @@ export const useSettings = create((set, get) => ({ const raw = await SecureStore.getItemAsync(SETTINGS_KEY) if (raw) { const parsed = JSON.parse(raw) as Partial - // Merge stored notifications with defaults so new categories get their default - const notifications = { ...DEFAULTS.notifications, ...parsed.notifications } - set({ ...DEFAULTS, ...parsed, notifications, loaded: true }) + // Merge stored settings with defaults so new fields/categories get their default + set({ ...mergeStoredSettings(DEFAULTS, parsed), loaded: true }) return } set({ loaded: true }) }, setPageSize: async (size) => { - const clamped = Math.max(10, Math.min(200, size)) + const clamped = clampPageSize(size) set({ pageSize: clamped }) await persist({ ...snapshot(get), pageSize: clamped }) },